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.
