Re: PATCH? rcu_do_batch: fix a pure theoretical memory ordering race

[Eric Dumazet <dada1@cosmosbay.com>]

Oleg Nesterov a écrit :
> On top of rcu-add-a-prefetch-in-rcu_do_batch.patch
> 
> rcu_do_batch:
> 
> 	struct rcu_head *next, *list;
> 
> 	while (list) {
> 		next = list->next;	<------ [1]
> 		list->func(list);
> 		list = next;
> 	}
> 
> We can't trust *list after list->func() call, that is why we load list->next
> beforehand. However I suspect in theory this is not enough, suppose that
> 
> 	- [1] is stalled
> 
> 	- list->func() marks *list as unused in some way
> 
> 	- another CPU re-uses this rcu_head and dirties it
> 
> 	- [1] completes and gets a wrong result
> 
> This means we need a barrier in between. mb() looks more suitable, but I think
> rmb() should suffice.
> 

Well, hopefully the "list->func()" MUST do the right thing [*], so your patch 
is not necessary.

For example, most structures are freed with kfree()/kmem_cache_free() and 
these functions MUST imply an smp_mb() [if/when exchanging data with other 
cpus], or else many uses in the kernel should be corrected as well.


[*] : In particular, slab code managment does something special when 
transfering local objects from local cpu A store to 'other cpus B'.
Other mechanisms should also use some kind of memory barrier in order to 
transfer an object to another cpu too, or you could imagine in flight stores 
from CPU A overwriting an object that was 'given' to CPU B.