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

[Carlos O'Donell <carlos_odonell at mentor dot com>]

On 9/24/2012 2:30 AM, Peng Haitao wrote:
> 
> On 09/20/2012 02:25 AM, Mike Frysinger wrote:
>>> This patch is unacceptable for two reasons:
>>>
>>> (a) We define bindresvport() as non-thread-safe. Therefore the change
>>> is not required.
>>>
>>> and
>>>
>>> (b) Adding locking to bindresvport() will only slow down the fast path
>>> in the function.
>>>
>>> You have provided no rationale for the change.
>>>
>>> Until you provide some rationale, or prove that performance doesn't
>>> matter in the case of this function, the change is unacceptable.
>>>
>>> Does that make sense?
>>
>> rather than adding locks, what if the static's had __thread added, and getpid 
>> was changed to gettid.
>> -mike
>>
> 
> gettid()'s manpage said: gettid() is Linux-specific and should not be used
> in programs that are intended to be portable.
> 
> It is rightly using gettid() in bindresvport()?

No.

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.

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.

Cheers,
Carlos.
-- 
Carlos O'Donell
Mentor Graphics / CodeSourcery
carlos_odonell@mentor.com
carlos@codesourcery.com
+1 (613) 963 1026