Messages in this thread | | | From | David Howells <> | Subject | Re: [PATCH] NFS: Fix RCU warnings in nfs_inode_return_delegation_noreclaim() [ver #2] | Date | Tue, 30 Mar 2010 17:37:49 +0100 |
| |
Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> rcu: Add update-side variant of rcu_dereference() > > Upcoming consistency-checking features are requiring that even update-side > accesses to RCU-protected pointers use some variant of rcu_dereference(). > Even though rcu_dereference() is quite lightweight, it does constrain the > compiler, thus producing code that is worse than required. This patch > therefore adds rcu_dereference_update(), which allows lockdep-style > checks for holding the correct update-side lock, but which does not > constrain the compiler.
Ummm... I'm not so keen on the name for two reasons. Firstly, why shouldn't the read side do:
struct foo { struct bar *b; };
void manage_bar(struct foo *f) { struct bar *b;
rcu_read_lock(); b = rcu_dereference(f->b); if (b) do_something_to_bar(b); rcu_read_unlock(); }
void manage_foo(struct foo *f) { ... if (f->b) manage_bar(f); ... }
Why should this be limited to the update side?
Secondly, the name rcu_dereference_update() seems to imply that this function itself does an update, perhaps after having done an rcu_dereference().
Perhaps rcu_pointer_valid()?
if (rcu_pointer_valid(f->b)) manage_bar(f);
or if you really do want to limit this sort of thing to the update side:
if (rcu_destination_for_update(f->b)) { spin_lock(&f->lock); update_bar(f); spin_unlock(&f->lock); }
Another possibility is have an 'RCU write lock' that just does the lockdep thing and doesn't interpolate a barrier:
rcu_write_lock(); if (rcu_dereference_for_update(f->b)) { spin_lock(&f->lock); update_bar(f->b); spin_unlock(&f->lock); } rcu_write_unlock();
Or might it make sense to roll together with the lock primitive:
if (rcu_dereference_and_lock(f->b, &f->lock)) { update_bar(f); spin_unlock(&f->lock); }
(I'm not keen on that one because you might not want to take the lock immediately, and you have a wide choice of locks).
Sorry to be picky.
David
| |