Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
