RFC change sb_writers to use percpu_rw_semaphore
Get series with b4
b4 am [email protected]Versions newest first
- v1 RFC Latest4 patches 22 repliesIn review
Patches in v1 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/4 | change get_super_thawed() to use sb_start/end_write()fs/ | +2 | 2 replies | |
| 2/4 | introduce sb_unlock_frozen()fs/ | +8 | 0 replies | |
| 3/4 | introduce sb_lockdep_release()fs/ | +11 | 0 replies | |
| 4/4 | change sb_writers to use percpu_rw_semaphorefs/ include/ | +47 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [PATCH RFC 0/4] change sb_writers to use percpu_rw_semaphore
Hello,
Al, Jan, could you comment? I mean the intent, the patches are obviously not for inclusion yet.
We can remove everything from struct sb_writers except frozen (which can become a boolean, it seems) and add the array of percpu_rw_semaphore's instead.
__sb_start/end_write() can use percpu_down/up_read(), and freeze/thaw_super() can use percpu_down/up_write().
Why:
- Firstly, __sb_start_write() looks simply buggy. I does __sb_end_write() if it sees ->frozen, but if it migrates to another CPU before percpu_counter_dec() sb_wait_write() can wrongly succeed if there is another task which holds the same "semaphore": sb_wait_write() can miss the result of the previous percpu_counter_inc() but see the result of this percpu_counter_dec().
- This code doesn't look simple. It would be better to rely on the generic locking code.
- __sb_start_write() will be a little bit faster, but this is minor.
Todo:
- __sb_start_write(wait => false) always fail.
Thivial, we already have percpu_down_read_trylock() just this patch wasn't merged yet.
- sb_lockdep_release() and sb_lockdep_acquire() play with percpu_rw_semaphore's internals.
Trivial, we need a couple of new helper in percpu-rwsem.c.
- Fix get_super_thawed(), it will spin if MS_RDONLY...
It is not clear to me what exactly should we do, but this doesn't look hard. Perhaps it can just return if MS_RDONLY.
- Most probably I missed something else, and I do not need how to test.
Finally. freeze_super() calls synchronize_sched_expedited() 3 times in a row. This is bad and just stupid. But if we change percpu_rw_semaphore to use rcu_sync (see https://lkml.org/lkml/2015/7/11/211) we can avoid this and do synchronize_sched() only once. Just we need some more simple changes in percpu-rwsem.c, so that all sb_writers->rw_sem[] semaphores could use the single sb_writers->rss.
In this case destroy_super() needs some modifications too, percpu_free_rwsem() will be might_sleep(). But this looks simple too.
Oleg.
fs/super.c | 147 +++++++++++++++++++-------------------------------- include/linux/fs.h | 14 +---- 2 files changed, 58 insertions(+), 103 deletions(-)