Re: [PATCH v2] Make bindresvport() function to multithread-safe

[Peng Haitao <penght at cn dot fujitsu dot com>]

On 09/24/2012 11:08 PM, Carlos O'Donell wrote:
> Define a macro in an internal header that for generic unix uses __getpid,
> and for Linux it is overriden by a linux-specific header that defines
> that macro as a combination of either INTERNAL_SYSCALL/INLINE_SYSCALL.
> 

Thanks.
I have sent v4 patch, please review.

> The whole point of that code is to create a hash function from which
> to start looking for free ports. If the hash function has poor spread
> because you only have __getpid in your OS, then that's not a serious
> issue.
> 
> You will still need to present performance data to show how these changes
> effect the performance of the original non-thread-safe version.
> 

The single-thread test result is as follows:
Before the patch, execute the test program with 100 times:

# perf stat -r 100 -e instructions -- ./bindresvport_test > /dev/null

 Performance counter stats for './bindresvport_test' (100 runs):

       106,594,501 instructions              #    0.00  insns per cycle          ( +-  0.35% )

       0.028158698 seconds time elapsed                                          ( +-  0.88% )


After the patch, execute the test program with 100 times:

# perf stat -r 100 -e instructions -- ./bindresvport_test > /dev/null

 Performance counter stats for './bindresvport_test' (100 runs):

       104,938,026 instructions              #    0.00  insns per cycle          ( +-  0.44% )

       0.027621324 seconds time elapsed                                          ( +-  0.94% )



The multi-threaded test result is as follows:
Before the patch, execute the test program with 100 times:

# perf stat -r 100 -e instructions -- ./bindresvport_mul_test > /dev/null
bindresvport: Address already in use
...

 Performance counter stats for './bindresvport_mul_test' (100 runs):

       116,481,478 instructions              #    0.00  insns per cycle          ( +-  0.25% )

       0.023839225 seconds time elapsed                                          ( +-  2.04% )


After the patch, execute the test program with 100 times:
# perf stat -r 100 -e instructions -- ./bindresvport_mul_test > /dev/nullbindresvport: Address already in use
bindresvport: Address already in use
...

 Performance counter stats for './bindresvport_mul_test' (100 runs):

       124,069,053 instructions              #    0.00  insns per cycle          ( +-  0.32% )

       0.021486935 seconds time elapsed                                          ( +-  0.84% )


The single-thread test program is bindresvport_test.c
The multi-thread test program is bindresvport_mul_test.c


-- 
Best Regards,
Peng

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <sys/types.h>
#include <netinet/in.h>
#include <errno.h>
#include <strings.h>
#include <arpa/inet.h>

#define THREADNUM 10
#define PORTNUM   51

void *thread_routine(void *arg)
{
	int sockfd, i, ret;
	struct sockaddr_in addr;

	bzero ((char *) &addr, sizeof (addr));
	addr.sin_family = AF_INET;

	for (i = 0; i < PORTNUM; i++)
	{
		if ((sockfd = socket(AF_INET, SOCK_STREAM, 0)) == -1)
		{
			perror("socket");
			return NULL;
		}
		if ((ret = bindresvport(sockfd, &addr)) == -1)
			perror("bindresvport");
		else
			printf("port = %u\n", htons(addr.sin_port));
	}

	return NULL;
}

int main (int argc, char *argv[])
{
	pthread_t thread_id[THREADNUM];
	void *thread_result[THREADNUM];
	int status, i;

	for(i = 0; i < THREADNUM; i++) {
		status = pthread_create(&thread_id[i], NULL, thread_routine, NULL);
		if (status != 0)
			perror("pthread_create");
	}

	for(i = 0; i < THREADNUM; i++) {
		status = pthread_join(thread_id[i], &thread_result[i]);
		if (status != 0)
			perror("pthread_join");
	}

	return 0;
}

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <netinet/in.h>
#include <errno.h>
#include <strings.h>
#include <arpa/inet.h>

#define THREADNUM 10
#define PORTNUM   51

void *thread_routine(void)
{
	int sockfd, i, ret;
	struct sockaddr_in addr;

	bzero ((char *) &addr, sizeof (addr));
	addr.sin_family = AF_INET;

	for (i = 0; i < PORTNUM; i++)
	{
		if ((sockfd = socket(AF_INET, SOCK_STREAM, 0)) == -1)
		{
			perror("socket");
			return NULL;
		}
		if ((ret = bindresvport(sockfd, &addr)) == -1)
			perror("bindresvport");
		else
			printf("port = %u\n", htons(addr.sin_port));
	}

	return NULL;
}

int main (int argc, char *argv[])
{
	int i;

	for(i = 0; i < THREADNUM; i++) {
		thread_routine();
	}

	return 0;
}