From: David Howells on
Paul E. McKenney <paulmck(a)linux.vnet.ibm.com> wrote:

> The new rcu_access_pointer() primitive is for the case where the pointer
> is be fetch and not dereferenced. This primitive may be used without
> protection, RCU or otherwise, due to the fact that it uses ACCESS_ONCE().
> ...
> +#define rcu_access_pointer(p, c) \

NAK. This shouldn't have the conditional parameter 'c'. Given that 'c' (by
analogy to rcu_dereference_check()) is there to describe the conditions under
which it's permitted to dereference the pointer, why is that relevant here?
What is it you're proving?

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Howells on
Paul E. McKenney <paulmck(a)linux.vnet.ibm.com> wrote:

> In other cases, there will be a reference counter or a "not yet fully
> initialized" flag that can (and should) be tested.

Why would you be using rcu_access_pointer() there? Why wouldn't you be using
rcu_dereference_protected()?


Also, one other thing: Should the default versions of these functions make
some reference to 'c' to prevent compiler warnings? Should:

#define rcu_dereference_check(p, c) rcu_dereference_raw(p)

for example, be:

#define rcu_dereference_check(p, c) \
({ \
if (1 || !(c)) \
rcu_dereference_raw(p); \
})

I'm not sure it's necessary, but it's possible to envisage a situation where
someone calculates something specifically for use in 'c', which will cause an
warning from the compiler if c isn't then checked.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Howells on
Paul E. McKenney <paulmck(a)linux.vnet.ibm.com> wrote:

> And all of the examples I could come up with that had c!=1 were contorted,
> even by my standards. So you were right, and I will drop the "c" on my
> next set of patches.

:-)

When it's done, I'll rebuild my keys and NFS patches on top of it.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Howells on
Paul E. McKenney <paulmck(a)linux.vnet.ibm.com> wrote:

> +#define rcu_access_pointer(p) ((void *)ACCESS_ONCE(p))
> ...
> +#define rcu_access_pointer(p) ((void *)ACCESS_ONCE(p))

There's no difference between your two versions of rcu_access_pointer(), so
you could move that whole construct outside of the #ifdef'ed section.

Other than that:

Acked-by: David Howells <dhowells(a)redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Howells on
Paul E. McKenney <paulmck(a)linux.vnet.ibm.com> wrote:

> This patch adds variants of rcu_dereference() that handle situations
> where the RCU-protected data structure cannot change, perhaps due to
> our holding the update-side lock, or where the RCU-protected pointer is
> only to be fetched, not dereferenced. These are needed due to some
> performance concerns with using rcu_dereference() where it is not
> required, aside from the need for lockdep/sparse checking.
>
> The new rcu_access_pointer() primitive is for the case where the pointer
> is be fetch and not dereferenced. This primitive may be used without
> protection, RCU or otherwise, due to the fact that it uses ACCESS_ONCE().
>
> The new rcu_dereference_protected() primitive is for the case where updates
> are prevented, for example, due to holding the update-side lock. This
> primitive does neither ACCESS_ONCE() nor smp_read_barrier_depends(), so
> can only be used when updates are somehow prevented.
>
> Suggested-by: David Howells <dhowells(a)redhat.com>
> Signed-off-by: Paul E. McKenney <paulmck(a)linux.vnet.ibm.com>

Acked-by: David Howells <dhowells(a)redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/