Page MenuHomeFreeBSD

Reduce chance of RCU deadlock in the LinuxKPI by implementing the section feature of the concurrency kit, CK.
ClosedPublic

Authored by hselasky on Sun, Mar 28, 7:41 AM.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sys/compat/linuxkpi/common/include/linux/sched.h
84

rcu_section should have ck_epoch_section_t[] type and not unsigned int[].

If there are issues with inclusion of ck_epoch.h into sched.h, then some way to ensure the type equality should be established.

sys/compat/linuxkpi/common/src/linux_rcu.c
202

You should assert that the increment does not overflow.

240

You need to assert that rcu_recurse[type] > 0 before decrement.

254

This container_of() was quite puzzling to me until I realized that it is in fact a type cast. See my note near rcu_section[].

hselasky added inline comments.
sys/compat/linuxkpi/common/include/linux/sched.h
84

See my comment in the end.

sys/compat/linuxkpi/common/src/linux_rcu.c
254

Changed into a regular cast. The problem is that EPOCH header files do not live under sys/ and require special C-flags currently.

kib added inline comments.
sys/compat/linuxkpi/common/src/linux_rcu.c
93

New code should prefer _Static_assert() over CTASSERT().

This revision is now accepted and ready to land.Sun, Mar 28, 2:50 PM

Why exactly does the RCU implementation not use epoch(9) directly?

sys/compat/linuxkpi/common/src/linux_rcu.c
222–224

Is this line too long?

Why exactly does the RCU implementation not use epoch(9) directly?

@markj: Because EPOCH(9) is not sleepable.

This revision now requires review to proceed.Sun, Mar 28, 3:37 PM
hselasky added inline comments.
sys/compat/linuxkpi/common/src/linux_rcu.c
93

The style in LinuxKPI is CTASSERT() for now.

Why exactly does the RCU implementation not use epoch(9) directly?

@markj: Because EPOCH(9) is not sleepable.

Neither is RCU. "Sleepable" RCU corresponds to "bounded sleep" in FreeBSD (per the definition in locking.9), and preemptible epoch sections permit that already. Do we have some code which expects to perform unbounded sleep in an SRCU read section?

The change looks ok to me in any case.

This revision is now accepted and ready to land.Sun, Mar 28, 4:50 PM

Neither is RCU. "Sleepable" RCU corresponds to "bounded sleep" in FreeBSD (per the definition in locking.9), and preemptible epoch sections permit that already.

Even if this problem is solved, there would be an issue with the epoch tracker structure, that RCU has no such equivalent.
Keep in mind that the LinuxKPI "lowers" mutexes one level. spinlock->mutex, mutex->sxlock and so on.

I need to check if Linux allows mutexes inside RCU. If so, we need sleepable support.

Do we have some code which expects to perform unbounded sleep in an SRCU read section?

There is some code which use SRCU, and it can sleep for a while, blocking the synchronize function of course.
The logic in RCU in FreeBSD is a bit different than with EPOCH. I understand it would be great if the two could merge, but I'm not sure how.

--HPS