On 18.03.19 г. 21:29 ч., David Sterba wrote: > On Thu, Mar 14, 2019 at 09:26:37AM +0200, Nikolay Borisov wrote: >> >> >> On 13.03.19 г. 17:46 ч., David Sterba wrote: >>> The series moves several atomic counters under CONFIG_BTRFS_DEBUG. The >>> selected counters are not essential for the extent buffer locking to >>> work. There's some space saving (4x 4B at least) and the cachelines are >>> less stressed on non-debugging builds. >>> >>> The final size is 264 from 280, getting to 256 would be nice but hard or >>> making the code unreadable. I have ideas to shave 7 more bytes but >>> that's for another patchset. >>> >>> David Sterba (9): >>> btrfs: add assertion helpers for spinning writers >>> btrfs: use assertion helpers for spinning writers >>> btrfs: add assertion helpers for spinning readers >>> btrfs: use assertion helpers for spinning readers >>> btrfs: add assertion helpers for extent buffer read lock counters >>> btrfs: use assertion helpers for extent buffer read lock counters >>> btrfs: add assertion helpers for extent buffer write lock counters >>> btrfs: use assertion helpers for extent buffer write lock counters >>> btrfs: switch extent_buffer::lock_nested to bool >>> >>> fs/btrfs/extent_io.c | 13 +++-- >>> fs/btrfs/extent_io.h | 11 ++-- >>> fs/btrfs/locking.c | 133 ++++++++++++++++++++++++++++++------------- >>> 3 files changed, 107 insertions(+), 50 deletions(-) >> >> Overall the series looks good, one thing I wonder is wouldn't it be >> better if you convert those WARN_ON to assert so that we fail fast in >> case the locking invariant are broken? > > This probably makes sense, the patches copy the existing code without > other updates. As it is entirely under debug ifdef, BUG_ON would work > too so it's not dependent on asserts compiled in or not. I'm fine with turning it into a BUG_ON as well. >
