tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: sysctl_doeproc() race



On 11.03.2018 19:20, Kamil Rytarowski wrote:
> On 11.03.2018 18:23, Robert Elz wrote:
>>     Date:        Sun, 11 Mar 2018 16:02:57 +0100
>>     From:        Kamil Rytarowski <n54%gmx.com@localhost>
>>     Message-ID:  <10b0efe2-35bb-d1e8-ed28-587f643bdfd7%gmx.com@localhost>
>>
>>   | We already use markers so I prefer to stick to this solution and it
>>   | makes this code reliable.
>>
>> Actually, since I sent the previous message, I have been thinking on it
>> more, and I don't think that it does.
>>
>> It handles the case where we have found a process on allproc, and it is
>> (while we are scanning more of allproc) moved to zombproc, so we won't
>> get the process twice.
>>
> 
> That was the original design.
> 
> I will have a look at the approach with iterating over pid_table[].
> 

The approach with iterating over pid_table[] didn't work well for me and
I've abandoned it quickly.

I've been testing a patch splitting KERN_PROC_PID out into a new
branch/function:

http://netbsd.org/~kamil/patch-00046-zombielist-marker-sysctl_doeproc-split.txt

My impression is that these changes make the code much less readable,
and the performance gain.... was negligible or even negative (!) in a
regular running systems with 20-30 processes. I expect that such
interface is clearly not a quick-path.

I was testing the proposal by Christos swapping zombie and allproc lists
and I've found that this code is stable for the original problem.

http://netbsd.org/~kamil/patch-00046-zombielist-marker-sysctl_doeproc-swap.txt

I've included in this patch additionally:
 - Short-circuit break for KERN_PROC_PID.
 - Removal of redundant "if (kbuf)" and "if (marker)" checks.
 - Update of comments regarding potential optimization, explaining why
we don't want it.
 - ESRCH returned for badly written software, that is not prepared to
see no result (or see race). This also makes the case of replying a call
easier to detect.

I've not observed any ATF regression in local tests and I'm going to
commit this code swapping allproc and zombieproc order of checking now.

Attachment: signature.asc
Description: OpenPGP digital signature



Home | Main Index | Thread Index | Old Index