Messages in this thread | | | Date | Sun, 3 Dec 2006 23:01:53 +0300 | From | Oleg Nesterov <> | Subject | Re: PATCH? rcu_do_batch: fix a pure theoretical memory ordering race |
| |
On 12/03, Eric Dumazet wrote: > > 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.
Yes, I don't claim it is necessary, note the "pure theoretical".
> 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.
Yes, mb() is enough (wmb() isn't) and kfree()/kmem_cache_free() are ok. And I don't know any example of "unsafe" code in that sense.
However I believe it is easy to make the code which is correct from the RCU's API pov, but unsafe.
Oleg.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |