On Sun, Nov 6, 2016 at 12:38 AM, Andi Kleen <a...@firstfloor.org> wrote: > Vitaly Wool <vitalyw...@gmail.com> writes: > >> Most of z3fold operations are in-page, such as modifying z3fold >> page header or moving z3fold objects within a page. Taking >> per-pool spinlock to protect per-page objects is therefore >> suboptimal, and the idea of having a per-page spinlock (or rwlock) >> has been around for some time. However, adding one directly to the >> z3fold header makes the latter quite big on some systems so that >> it won't fit in a signle chunk. > >> + atomic_t page_lock; > > This doesnt make much sense. A standard spinlock is not bigger > than 4 bytes either. Also reinventing locks is usually a bad > idea: they are tricky to get right, you have no debugging support, > hard to analyze, etc.
I understand the reinvention part but you're not quite accurate here with the numbers. E. g. on x86_64: (gdb) p sizeof(rwlock_t) $1 = 8 I believe a DIY lock is justified here, since the variant with rwlock_t actually caused complaints from kbuild test robot building the previous version of this patch [1] with gcc-6.0 for x86_64: In file included from arch/x86/include/asm/atomic.h:4:0, from include/linux/atomic.h:4, from mm/z3fold.c:25: mm/z3fold.c: In function 'init_z3fold': >> include/linux/compiler.h:518:38: error: call to '__compiletime_assert_808' >> declared with attribute error: BUILD_BUG_ON failed: sizeof(struct >> z3fold_header) > ZHDR_SIZE_ALIGNED ~vitaly [1] https://patchwork.kernel.org/patch/9384871/