Re: [PATCH] proc: readdir race fix.

[ebiederm@xmission.com (Eric W. Biederman)]

KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> writes:

> Hi,
>
> On Mon, 04 Sep 2006 17:13:10 -0600
> ebiederm@xmission.com (Eric W. Biederman) wrote:
>> These better semantics are implemented by scanning through the
>> pids in numerical order and by making the file offset a pid
>> plus a fixed offset.
> I think this is very sane/solid approach.
> Maybe this is the way to go. I'll test and ack later, thank you.
>
>> The pid scan happens on the pid bitmap, which when you look at it is
>> remarkably efficient for a brute force algorithm.  Given that a typical
>> cache line is 64 bytes and thus covers space for 64*8 == 200 pids.  There
>> are only 40 cache lines for the entire 32K pid space.  A typical system
>> will have 100 pids or more so this is actually fewer cache lines we have
>> to look at to scan a linked list, and the worst case of having to scan
>> the entire pid bitmap is pretty reasonable.
>
> I agree with you but..
> Becasue this approach has to access *all* task structs in a system,
> and have to scan pidhash many times. I'm not sure that this scan & lookup
> is good for future implementation. But this patch is obviously better than
> current implementation.

Quite possibly.  But where this approach falls down in not in the basics
but in the data structures.  And it isn't that hard to fix the data
structures.  Just something I don't want to do the first pass.

>>  /*
>> + * Used by proc to find the pid with the first
>> + * pid that is greater than or equal to number.
>> + *
>> + * If there is a pid at nr this function is exactly the same as find_pid.
>> + */
>> +struct pid *find_next_pid(int nr)
>> +{
>
> How about find_first_used_pid(int nr) ?

Maybe.  The best name I can think of is:
find_greater_or_equal_pid(int nr) and that is a little 
clumsy.   On the other hand it isn't confusing what the function
does.

Any other suggestions?

Eric