Re: [PATCH] kasan: fix races in quarantine_remove_cache()

[Andrey Ryabinin <aryabinin@virtuozzo.com>]

On 03/09/2017 12:37 PM, Dmitry Vyukov wrote:
>>>  void quarantine_reduce(void)
>>>  {
>>>       size_t total_size, new_quarantine_size, percpu_quarantines;
>>>       unsigned long flags;
>>> +     int srcu_idx;
>>>       struct qlist_head to_free = QLIST_INIT;
>>>
>>>       if (likely(READ_ONCE(quarantine_size) <=
>>>                  READ_ONCE(quarantine_max_size)))
>>>               return;
>>>
>>> +     /*
>>> +      * srcu critical section ensures that quarantine_remove_cache()
>>> +      * will not miss objects belonging to the cache while they are in our
>>> +      * local to_free list. srcu is chosen because (1) it gives us private
>>> +      * grace period domain that does not interfere with anything else,
>>> +      * and (2) it allows synchronize_srcu() to return without waiting
>>> +      * if there are no pending read critical sections (which is the
>>> +      * expected case).
>>> +      */
>>> +     srcu_idx = srcu_read_lock(&remove_cache_srcu);
>>
>> I'm puzzled why is SRCU, why not RCU? Given that we take spin_lock in the next line
>> we certainly don't need ability to sleep in read-side critical section.
> 
> I've explained it in the comment above.
 
I've read it. It doesn't explain to me why is SRCU is better than RCU here.
 a) We can't sleep in read-side critical section. Given that RCU is almost always
	faster than SRCU, RCU seem preferable.
 b) synchronize_rcu() indeed might take longer to complete. But does it matter?
    We to synchronize_[s]rcu() only on cache destruction which relatively rare operation and 
    it's not a hotpath. Performance of the quarantine_reduce() is more important

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>