lkml.org 
[lkml]   [2019]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v6 3/5] locking/qspinlock: Introduce CNA into the slow path of qspinlock
From
Date


> On Nov 20, 2019, at 10:16 AM, kbuild test robot <lkp@intel.com> 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@intel.com>
>
> 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

\
 
 \ /
  Last update: 2019-11-22 19:31    [W:0.194 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site