Re: [PATCH RFC] time: drop do_sys_times spinlock

[Rik van Riel <riel@redhat.com>]

On 08/13/2014 02:45 PM, Oleg Nesterov wrote:
> On 08/13, Rik van Riel wrote:
>>
>> On 08/13/2014 02:08 PM, Oleg Nesterov wrote:
>>>
>>> Well, I disagree. This is more complex, and this adds yet another lock
>>> which only protects the stats...
>>
>> The other lock is what can tell us that there is a writer active
>> NOW, which may be useful when it comes to guaranteeing forward
>> progress for readers when there are lots of threads exiting...
> 
> I don't really understand why seqcount_t is better in this sense, either
> way we need to to taking the lock if we want to guarantee a forward
> progress. read_seqbegin_or_lock() doesn't even work "automagically",
> and it can't be used in this case anyway.

It allows subsequent readers to fall back into lockless mode,
once the first reader (that got blocked behind writers) takes
the lock, temporarily locking out writers.

This protects forward progress, without the danger of
permanently degrading throughput due to increased contention.

> That said, it is not that I am really sure that seqcount_t in ->signal
> is actually worse, not to mention that this is subjective anyway. IOW,
> I am not going to really fight with your approach ;)

I agree that both approaches have their advantages and
disadvantages.

>>> Whatever we do, we should convert thread_group_cputime() to use
>>> for_each_thread() first().
>>
>> What is the advantage of for_each_thread over while_each_thread,
>> besides getting rid of that t = tsk line?
> 
> It is buggy and should die, see 0c740d0afc3bff0a097ad.

I just got rid of it in the code that I touched.

Thanks for pointing it out.