Re: A racy reading spot on n->free_objects in slab.c

[Vlastimil Babka <vbabka@suse.cz>]

On 4/14/21 9:32 AM, Christoph Lameter wrote:
> On Tue, 13 Apr 2021, Gong, Sishuai wrote:
> 
>> We found a racy reading spot on shared variable n->free_objects in

I'm assuming this was found with some research tool you're developing?
Did it also flag the line "shared = READ_ONCE(n->shared);" as that's basically
the same thing.

>> slab.c and it can be data-racing with several writers that update this
>> variable. As shown below, in function cache_alloc_refill(),
>> n->free_objects will be read without any protection. It could be
>> possible that the read value immediately becomes out-of-date when
>> another writer is changing it (e.g. free_block())
> 
> Ok that is fine. If we mistakenly fill up the per cpu cache with new
> objects to the slab then so be it.

> If we mistakenly take the lock and fail to get an object then we can still
> reverse that decision and do the other thing.

Agreed. It's common in the kernel to do optimistic reads outside of lock to
decide what to do and avoid locking at all in some cases. This may sacrifice
some precision of these decisions. but not correctness, as locks are taken later
for the critical parts.

> Maybe we need to add a comment there?

Or maybe some construct that makes no difference for the compiler, but does for
the tool? READ_ONCE() maybe?