> On Nov 22, 2019, at 2:29 PM, Waiman Long <longman@xxxxxxxxxx> wrote: > > On 11/22/19 1:28 PM, Alex Kogan wrote: >> >>> On Nov 20, 2019, at 10:16 AM, kbuild test robot <lkp@xxxxxxxxx> wrote: >>> >>> Hi Alex, >>> >>> Thank you for the patch! Yet something to improve: >>> >>> [auto build test ERROR on linus/master] >>> [also build test ERROR on v5.4-rc8 next-20191120] >>> [if your patch is applied to the wrong git tree, please drop us a note to help >>> improve the system. BTW, we also suggest to use '--base' option to specify the >>> base tree in git format-patch, please see https://urldefense.proofpoint.com/v2/url?u=https-3A__stackoverflow.com_a_37406982&d=DwIBAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=Hvhk3F4omdCk-GE1PTOm3Kn0A7ApWOZ2aZLTuVxFK4k&m=BxEt1232ccGlMGDinAB0QAUaTFyl-m5sp4C-crHjpoU&s=OzzQqg4fTDV55X-y4vbnGeXoJaPHSvO_EfrUQnMVRHc&e= ] >>> >>> url: https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_0day-2Dci_linux_commits_Alex-2DKogan_locking-2Dqspinlock-2DRename-2Dmcs-2Dlock-2Dunlock-2Dmacros-2Dand-2Dmake-2Dthem-2Dmore-2Dgeneric_20191109-2D180535&d=DwIBAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=Hvhk3F4omdCk-GE1PTOm3Kn0A7ApWOZ2aZLTuVxFK4k&m=BxEt1232ccGlMGDinAB0QAUaTFyl-m5sp4C-crHjpoU&s=uE7ZeYXOFiu09PUVjnCntEe2rR5x_QxS6dEW9twpfok&e= >>> base: https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git&d=DwIBAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=Hvhk3F4omdCk-GE1PTOm3Kn0A7ApWOZ2aZLTuVxFK4k&m=BxEt1232ccGlMGDinAB0QAUaTFyl-m5sp4C-crHjpoU&s=aAKxuXc_c7OF0ffioQfVsIB6H-4Sd9PYxSM7kurm2ig&e= 0058b0a506e40d9a2c62015fe92eb64a44d78cd9 >>> config: i386-randconfig-f003-20191120 (attached as .config) >>> compiler: gcc-7 (Debian 7.4.0-14) 7.4.0 >>> reproduce: >>> # save the attached .config to linux build tree >>> make ARCH=i386 >>> >>> If you fix the issue, kindly add following tag >>> Reported-by: kbuild test robot <lkp@xxxxxxxxx> >>> >>> All error/warnings (new ones prefixed by >>): >>> >>> In file included from include/linux/export.h:42:0, >>> from include/linux/linkage.h:7, >>> from include/linux/kernel.h:8, >>> from include/linux/list.h:9, >>> from include/linux/smp.h:12, >>> from kernel/locking/qspinlock.c:16: >>> kernel/locking/qspinlock_cna.h: In function 'cna_init_nodes': >>>>> include/linux/compiler.h:350:38: error: call to '__compiletime_assert_80' declared with attribute error: BUILD_BUG_ON failed: sizeof(struct cna_node) > sizeof(struct qnode) >>> _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) >>> ^ >>> include/linux/compiler.h:331:4: note: in definition of macro '__compiletime_assert' >>> prefix ## suffix(); \ >>> ^~~~~~ >>> include/linux/compiler.h:350:2: note: in expansion of macro '_compiletime_assert' >>> _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) >>> ^~~~~~~~~~~~~~~~~~~ >>> include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert' >>> #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) >>> ^~~~~~~~~~~~~~~~~~ >>> include/linux/build_bug.h:50:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' >>> BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) >>> ^~~~~~~~~~~~~~~~ >>>>> kernel/locking/qspinlock_cna.h:80:2: note: in expansion of macro 'BUILD_BUG_ON' >>> BUILD_BUG_ON(sizeof(struct cna_node) > sizeof(struct qnode)); >>> ^~~~~~~~~~~~ >> Consider the following definition of qnode: >> >> struct qnode { >> struct mcs_spinlock mcs; >> #if defined(CONFIG_PARAVIRT_SPINLOCKS) || defined(CONFIG_NUMA_AWARE_SPINLOCKS) >> long reserved[2]; >> #endif >> }; >> >> and this is how cna_node is defined: >> >> struct cna_node { >> struct mcs_spinlock mcs; >> int numa_node; >> u32 encoded_tail; >> u32 pre_scan_result; /* 0, 1, 2 or encoded tail */ >> u32 intra_count; >> }; >> >> Since long is 32 bit on i386, we get the compilation error above. >> >> We can try and squeeze CNA-specific fields into 64 bit on i386 (or any 32bit >> architecture for that matter). Note that an encoded tail pointer requires up >> to 24 bits, and we have two of those. We would want different field encodings >> for 32 vs 64bit architectures, and this all will be quite ugly. >> >> So instead we should probably either change the definition of @reserved in qnode >> to long long, or perhaps disable CNA on 32bit architectures altogether? >> I would certainly prefer the former, especially as it requires the least amount >> of code/config changes. >> >> Any objections / thoughts? >> >> Thanks, >> — Alex >> > The easy way out is to restrict NUMA qspinlock to 64-bit only. There > aren't that many 32-bit NUMA systems out there that we have to worry about. > > Just add "depends on 64BIT" to the config entry. Ok, will do. Thanks, — Alex
