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

["Carlos O'Donell" <carlos at systemhalted dot org>]

On Tue, Oct 16, 2012 at 1:26 PM, Rich Felker <dalias@aerifal.cx> wrote:
> On Tue, Oct 16, 2012 at 05:33:51PM +0800, Peng Haitao wrote:
>>
>> On 09/28/2012 10:27 PM, Carlos O'Donell wrote:
>> >>  /*
>> >>   * Bind a socket to a privileged IP port
>> >>   */
>> >>  int
>> >>  bindresvport (int sd, struct sockaddr_in *sin)
>> >>  {
>> >> -  static short port;
>> >> +  static __thread short port;
>> >
>> > I'm curious how our relocation count changes with this?
>> >
>> > Net neutral or did we gain extra relocations by changing this from
>> > static to static __thread?
>> >
>> > Could you have a look into that?
>> >
>>
>> The static variable will cause bindresvport to unsafe.
>>
>> Use thread A and thread B to explain:
>>
>> thread A: when port is 1023, execute "if (port > endport)"
>> thread B: execute "sin->sin_port = htons (port++);", port is 1024
>> thread A: execute "res = __bind (sd, sin, sizeof (struct sockaddr_in));"
>>
>> So, static should be replaced with static __thread.
>
> Adding overhead to every thread in every program for the sake of a
> nonstandard function that will almost never be used is not a good
> design. Instead, it should simply be protected by a lock, or modified
> atomically.

The increase in glibc's .tdata size by a short?

I think that using __thread here increases concurrency in the library
without the cost of locking which can require a local CPU to take
ownership of an entire cache line. That cache line may contain more
than just the data we wanted to work with.

The two scenarios are:

(a) Mutex
- Decreased concurrency.
- Every program increase .data by sizeof(mutex) (maybe 40-50 bytes).
- Function is now thread safe.

(b) __thread
- Increased concurrency.
- Every thread increases .tdata by sizeof(short) (maybe 4 bytes with alignment).
- Function is now thread safe.

I don't see that the answer is that clear cut.

Given the future of lots and lots of cores I'd rather debug (b).

Rich, Why do you lean towards (a)?

Cheers,
Carlos.