lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a7eff2ed-0e80-edae-2a97-a3fdab2d05f5@google.com>
Date:   Fri, 1 Mar 2019 13:30:15 -0500
From:   Barret Rhoden <brho@...gle.com>
To:     Eial Czerwacki <eial@...lemp.com>, dennis@...nel.org,
        tj@...nel.org, cl@...ux.com
Cc:     linux-kernel@...r.kernel.org, Shai Fultheim <shai@...lemp.com>,
        Oren Twaig <oren@...lemp.com>
Subject: Re: [PATCH] percpu/module resevation: change resevation size iff
 X86_VSMP is set

Hi -

On 01/21/2019 06:47 AM, Eial Czerwacki wrote:
 >

Your main issue was that you only sent this patch to LKML, but not the 
maintainers of the file.  If you don't, your patch might get lost.  To 
get the appropriate people and lists, run:

	scripts/get_maintainer.pl YOUR_PATCH.patch.

For this patch, you'll get this:

Dennis Zhou <dennis@...nel.org> (maintainer:PER-CPU MEMORY ALLOCATOR)
Tejun Heo <tj@...nel.org> (maintainer:PER-CPU MEMORY ALLOCATOR)
Christoph Lameter <cl@...ux.com> (maintainer:PER-CPU MEMORY ALLOCATOR)
linux-kernel@...r.kernel.org (open list)

I added the three maintainers to this email.

I have a few minor comments below.

 > [PATCH] percpu/module resevation: change resevation size iff X86_VSMP 
is set

You misspelled 'reservation'.  Also, I'd just say: "percpu: increase 
module reservation size if X86_VSMP is set".  ('change' -> 'increase'), 
only says 'reservation' once.)

> as reported in bug #201339 (https://bugzilla.kernel.org/show_bug.cgi?id=201339)

I think you can add a tag for this right above your Signed-off-by tags. 
e.g.:

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201339

> by enabling X86_VSMP, INTERNODE_CACHE_BYTES's definition differs from the default one
> causing the struct size to exceed the size ok 8KB.
                                             ^of

Which struct are you talking about?  I have one in mind, but others 
might not know from reading the commit message.

I ran into this on https://bugzilla.kernel.org/show_bug.cgi?id=202511. 
In that case, it was because modules (drm and amdkfd) were using 
DEFINE_SRCU, which does a DEFINE_PER_CPU on struct srcu_data, and that 
used ____cacheline_internodealigned_in_smp.

> 
> in order to avoid such issue, increse PERCPU_MODULE_RESERVE to 64KB if CONFIG_X86_VSMP is set.
                                ^increase

> 
> the value was caculated on linux 4.20.3, make allmodconfig all and the following oneliner:
                ^calculated

> for f in `find -name *.ko`; do echo $f; readelf -S $f  |grep perc; done |grep data..percpu -B 1 |grep ko |while read r; do echo -n "$r: "; objdump --syms --section=.data..percpu $r|grep data |sort -n  |awk '{c++; d=strtonum("0x" $1) + strtonum("0x" $5); if (m < d) m = d;} END {printf("%d vars-> last addr 0x%x ( %d )\n", c, m, m)}' ; done |column -t |sort -k 8 -n | awk '{print $8}'| paste -sd+ | bc

Not sure how useful the one-liner is, versus a description of what 
you're doing.  i.e. "the size of all module percpu data sections, or 
something."

Also, how close was that calculated value to 64K?  If more modules start 
using DEFINE_SRCU, each of which uses 8K, then that 64K might run out.

Thanks,
Barret

> Signed-off-by: Eial Czerwacki <eial@...lemp.com>
> Signed-off-by: Shai Fultheim <shai@...lemp.com>
> Signed-off-by: Oren Twaig <oren@...lemp.com>
> ---
>   include/linux/percpu.h | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/percpu.h b/include/linux/percpu.h
> index 70b7123..6b79693 100644
> --- a/include/linux/percpu.h
> +++ b/include/linux/percpu.h
> @@ -14,7 +14,11 @@
>   
>   /* enough to cover all DEFINE_PER_CPUs in modules */
>   #ifdef CONFIG_MODULES
> +#ifdef X86_VSMP
> +#define PERCPU_MODULE_RESERVE		(1 << 16)
> +#else
>   #define PERCPU_MODULE_RESERVE		(8 << 10)
> +#endif
>   #else
>   #define PERCPU_MODULE_RESERVE		0
>   #endif
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ