MFC after: 1 week
Sponsored by: Mellanox Technologies // NVIDIA Networking
Details
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
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
| 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[]. | |
| sys/compat/linuxkpi/common/src/linux_rcu.c | ||
|---|---|---|
| 94 | New code should prefer _Static_assert() over CTASSERT(). | |
Comment Actions
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? | |
Comment Actions
Why exactly does the RCU implementation not use epoch(9) directly?
@markj: Because EPOCH(9) is not sleepable.
| sys/compat/linuxkpi/common/src/linux_rcu.c | ||
|---|---|---|
| 94 | The style in LinuxKPI is CTASSERT() for now. | |
Comment Actions
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.
Comment Actions
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