Re: [PATCH 1/2] rcu: add rcu_access_pointer and rcu_dereference_protect

David Howells
2010-04-07T16:19:58Z

Eric Dumazet <eric.dumazet@gmail.com> wrote:

> > You've missed the point.
>
> You already claimed I dont understand RCU. I find this claim funny.
>
> > For rcu_access_pointer(), _nothing_ protects the data, not only that, we
> > don't care: we're only checking the pointer.
>
> How can you state this ?
>
> Thats pretty simple, "always true" is a fine condition.
>
> What's the problem with this ?

If the condition for rcu_access_pointer() is always "always true", then it's
redundant, right? rcu_access_pointer() is for checking the pointer only, not
checking the payload that pointer might point to. So, what condition are you
supposed to be checking?
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> > but if 'c' is supposed to be the locks that protect the data, is this a
> > valid check?
>
> 'c' is not a lock. Its a condition.

Sorry, I meant the state of the relevant locking context.

To take your example:

> filter = rcu_dereference_check(sk->sk_filter,
> atomic_read(&sk->sk_wmem_alloc) == 0);

what is the value of sk->sk_wmem_alloc to the lock context of sk->sk_filter?
Why would lockdep be interested in sk_wmem_alloc?

Surely, the assertion that the value of sk->sk_filter is related to
sk_wmem_alloc being 0 is independent of the need to dereference sk_filter for
RCU purposes. So why are these being combined?

Why not:

ASSERT(atomic_read(&sk->sk_wmem_alloc) == 0);
filter = rcu_dereference(sk->sk_filter);

This is much clearer, and you're not combining an unrelated assertion with the
RCU dereference.

David