CODEWITHSUNDEEP
linuxlinux-kernelrcu
Yd Ahhrk
Yd Ahhrk

Reputation: 1138

How should I be mixing RCU reader and updater code?

I have a boilerplate function that finds a structure in a tree-like database:

struct foo {
    struct foo *child1; /* RCU-protected. */
    struct foo *child2; /* RCU-protected. */

    ... /* Other stuff */
}

static struct foo *find_foo(int arg)
{
    struct foo *parent;
    ... /* Do something to find parent. */
    return rcu_dereference(parent->child1); /* or child2. Whatever. */
}

And then I have several functions that do something with that result:

void a(int arg)
{
    struct foo *bar;
    rcu_read_lock();
    bar = find_foo(arg);
    ... /* Read something from bar and do something with it. */
    rcu_read_unlock();
}

And then I have a single updater/reclaimer. It needs to find the object just like the "a" functions (Assume it's already mutex'd from outside):

void c(int arg)
{
    struct foo *bar;

    bar = find_foo(arg);

    ... /* make a backup pointer of bar->child1. */

    rcu_assign_pointer(bar->child1, ...);

    ... /* free the old child or whatever. */
}

My problem is, rcu_dereference()'s documentation says it must be called from within a read-side critical section (ie. between rcu_read_lock() and rcu_read_unlock()). c() violates this rule by calling find_foo().

I'm hesitant to make a whole new version of find_foo() which uses rcu_dereference_protected() instead of rcu_dereference(), because it's too much duplicate code, so I'm wondering if this implementation of c() is legal:

void c(int arg)
{
    struct foo *bar;

    rcu_read_lock();
    bar = find_foo(arg);
    bar = rcu_dereference_protected(bar, ...); /* THIS. */
    rcu_read_unlock();

    ... /* make a backup pointer of bar->child1. */

    rcu_assign_pointer(bar->child1, ...);

    ... /* free the old child or whatever. */
}

If this is not legal, how should I instead be mixing reader and updater code?

Upvotes: 1

Views: 195

Answers (1)

Tsyvarev
Tsyvarev

Reputation: 66061

Actually, the first variant of c() is correct (no specific dereference is needed on updater side), but it makes rcu checker to be confused (the checker expects rcu_dereference occured under rcu_read section).

The second variant of c() is correct even from the view of rcu checker, but rcu_dereference_protected is not need: bar is already result of rcu_dereference.

Another way for make rcu checker happy is to use

rcu_dereference_check(<pointer>, <condition-when-in-updater>)

instead of

rcu_dereference(<pointer>)

inside foo implementation. So checker will not complain on foo() call outside of rcu_read section until it can prove that condition is false in that case.

With such foo() implementation the first variant of c() is OK.

Upvotes: 1

Related Questions