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

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

Details

Reviewers

markj
kib

Commits

rGb9de88350f2e: Reduce chance of RCU deadlock in the LinuxKPI by implementing the section
rG30b1f8cd1122: Reduce chance of RCU deadlock in the LinuxKPI by implementing the section
rG38e7de43ba24: Reduce chance of RCU deadlock in the LinuxKPI by implementing the section
rG177772088060: Reduce chance of RCU deadlock in the LinuxKPI by implementing the section

Summary

MFC after: 1 week
Sponsored by: Mellanox Technologies // NVIDIA Networking

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• hselasky created this revision.Mar 28 2021, 7:41 AM

Herald added subscribers: linuxkpi, emaste. · View Herald TranscriptMar 28 2021, 7:41 AM

• hselasky requested review of this revision.Mar 28 2021, 7:41 AM

kib added inline comments.Mar 28 2021, 1:39 PM

sys/compat/linuxkpi/common/include/linux/sched.h
85	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
203	You should assert that the increment does not overflow.
241	You need to assert that rcu_recurse[type] > 0 before decrement.
255	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[].

Address comments from @kib .

• hselasky marked 4 inline comments as done.Mar 28 2021, 2:13 PM

• hselasky added inline comments.

sys/compat/linuxkpi/common/include/linux/sched.h
85	See my comment in the end.
sys/compat/linuxkpi/common/src/linux_rcu.c
255	Changed into a regular cast. The problem is that EPOCH header files do not live under sys/ and require special C-flags currently.

kib accepted this revision.Mar 28 2021, 2:50 PM

kib added inline comments.

sys/compat/linuxkpi/common/src/linux_rcu.c
94	New code should prefer _Static_assert() over CTASSERT().

This revision is now accepted and ready to land.Mar 28 2021, 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.

Wrap some long lines.

This revision now requires review to proceed.Mar 28 2021, 3:37 PM

• hselasky marked 2 inline comments as done.Mar 28 2021, 3:37 PM

• hselasky added inline comments.

sys/compat/linuxkpi/common/src/linux_rcu.c
94	The style in LinuxKPI is CTASSERT() for now.

In D29467#660095, @hselasky wrote:

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.Mar 28 2021, 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

Closed by commit rG177772088060: Reduce chance of RCU deadlock in the LinuxKPI by implementing the section (authored by • hselasky). · Explain WhyMar 29 2021, 8:57 AM

This revision was automatically updated to reflect the committed changes.

• hselasky added a commit: rG177772088060: Reduce chance of RCU deadlock in the LinuxKPI by implementing the section.

• hselasky added a commit: rG38e7de43ba24: Reduce chance of RCU deadlock in the LinuxKPI by implementing the section.Apr 6 2021, 10:25 AM

• hselasky added a commit: rG30b1f8cd1122: Reduce chance of RCU deadlock in the LinuxKPI by implementing the section.Apr 6 2021, 10:28 AM

• hselasky added a commit: rGb9de88350f2e: Reduce chance of RCU deadlock in the LinuxKPI by implementing the section.