lkml.org 
[lkml]   [2006]   [Dec]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: PATCH? rcu_do_batch: fix a pure theoretical memory ordering race
    On 12/04, Eric Dumazet wrote:
    >
    > Oleg Nesterov a ?crit :
    > >
    > > int start_me_again;
    > >
    > > struct rcu_head rcu_head;
    > >
    > > void rcu_func(struct rcu_head *rcu)
    > > {
    > > start_me_again = 1;
    > > }
    > >
    > > // could be called on arbitrary CPU
    > > void check_start_me_again(void)
    > > {
    > > static spinlock_t lock;
    > >
    > > spin_lock(lock);
    > > if (start_me_again) {
    > > start_me_again = 0;
    > > call_rcu(&rcu_head, rcu_func);
    > > }
    > > spin_unlock(lock);
    > > }
    > >
    > >I'd say this code is not buggy.
    >
    > Are you sure ? Can you prove it ? :)

    Looks like you think differently :)

    > I do think your rcu_func() misses some sync primitive, *after*
    > start_me_again=1;
    > You seem to rely on some undocumented side effect.
    > Adding smp_rmb() before calling rcu_func() wont help.

    I guess you mean that check_start_me_again() can miss start_me_again != 0 ?
    Yes, of course, it should check the condition from time to time. We can even
    do
    start_me_again = 1;
    wake_up(&start_me_again_wq);

    , this is still unsafe.

    > >>A smp_rmb() wont avoid all possible bugs...
    > >
    > >For example?
    >
    > A smp_rmb() wont avoid stores pending on this CPU to be committed to memory
    > after another cpu takes the object for itself. Those stores could overwrite
    > stores done by the other cpu as well.

    Yes. But RCU core doesn't write to rcu_head (except call_rcu). Callback _owns_
    rcu_head, it should be ok to use it in any way without fear to break RCU.
    Of course, callback should take care of its own locking/ordering.

    > So in theory you could design a buggy callback function even after your
    > patch applied.

    So. Do you claim that rcu_func() above is buggy?

    > Any function that can transfer an object from CPU A scope to CPU B scope
    > must take care of memory barrier by itself. The caller *cannot* possibly do
    > the job, especially if it used an indirect call. However, in some cases it
    > is possible some clever algos are doing the reverse, ie doing the memory
    > barrier in the callers.
    >
    > Kernel is full of such constructs :
    >
    > for (ptr = head; ptr != NULL ; ptr = next) {
    > next = ptr->next;
    > some_subsys_delete(ptr);
    > }
    >
    > And we dont need to add smp_rmb() before the call to some_subsys_delete(),
    > it would be a nightmare, and would slow down modern cpus.

    Agreed.

    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/

    \
     
     \ /
      Last update: 2006-12-04 00:51    [W:0.023 / U:58.888 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site