* Re: [PATCH 2/4] hugetlbfs: move hugepagesz= parsing to arch independent code
From: Mike Kravetz @ 2020-03-19 17:00 UTC (permalink / raw)
To: Christophe Leroy, linux-mm, linux-kernel, linux-arm-kernel,
linuxppc-dev, linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Vasily Gorbik, Jonathan Corbet, Catalin Marinas,
Dave Hansen, Heiko Carstens, David S.Miller,
Christian Borntraeger, Ingo Molnar, Palmer Dabbelt, Paul Walmsley,
Paul Mackerras, Andrew Morton, Longpeng, Will Deacon,
Thomas Gleixner
In-Reply-To: <2ca058dc-47e6-1d08-154b-77d2cbe98e34@c-s.fr>
On 3/19/20 12:04 AM, Christophe Leroy wrote:
>
>
> Le 18/03/2020 à 23:06, Mike Kravetz a écrit :
>> Now that architectures provide arch_hugetlb_valid_size(), parsing
>> of "hugepagesz=" can be done in architecture independent code.
>> Create a single routine to handle hugepagesz= parsing and remove
>> all arch specific routines. We can also remove the interface
>> hugetlb_bad_size() as this is no longer used outside arch independent
>> code.
>>
>> This also provides consistent behavior of hugetlbfs command line
>> options. The hugepagesz= option should only be specified once for
>> a specific size, but some architectures allow multiple instances.
>> This appears to be more of an oversight when code was added by some
>> architectures to set up ALL huge pages sizes.
>>
>> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
>> ---
>> arch/arm64/mm/hugetlbpage.c | 15 ---------------
>> arch/powerpc/mm/hugetlbpage.c | 15 ---------------
>> arch/riscv/mm/hugetlbpage.c | 16 ----------------
>> arch/s390/mm/hugetlbpage.c | 18 ------------------
>> arch/sparc/mm/init_64.c | 22 ----------------------
>> arch/x86/mm/hugetlbpage.c | 16 ----------------
>> include/linux/hugetlb.h | 1 -
>> mm/hugetlb.c | 24 ++++++++++++++++++------
>> 8 files changed, 18 insertions(+), 109 deletions(-)
>>
>
> [snip]
>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 2f99359b93af..cd4ec07080fb 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -3149,12 +3149,6 @@ static int __init hugetlb_init(void)
>> }
>> subsys_initcall(hugetlb_init);
>> -/* Should be called on processing a hugepagesz=... option */
>> -void __init hugetlb_bad_size(void)
>> -{
>> - parsed_valid_hugepagesz = false;
>> -}
>> -
>> void __init hugetlb_add_hstate(unsigned int order)
>> {
>> struct hstate *h;
>> @@ -3224,6 +3218,24 @@ static int __init hugetlb_nrpages_setup(char *s)
>> }
>> __setup("hugepages=", hugetlb_nrpages_setup);
>> +static int __init hugepagesz_setup(char *s)
>> +{
>> + unsigned long long size;
>> + char *saved_s = s;
>> +
>> + size = memparse(s, &s);
>
> You don't use s after that, so you can pass NULL instead of &s and avoid the saved_s
Thanks!
I'll incorporate in v2.
--
Mike Kravetz
^ permalink raw reply
* Re: [PATCH v11 8/8] MAINTAINERS: perf: Add pattern that matches ppc perf to the perf entry.
From: Joe Perches @ 2020-03-19 17:03 UTC (permalink / raw)
To: Michal Suchanek, linuxppc-dev
Cc: Mark Rutland, Gustavo Luiz Duarte, Peter Zijlstra,
Sebastian Andrzej Siewior, linux-kernel, Paul Mackerras,
Jiri Olsa, Rob Herring, Michael Neuling, Mauro Carvalho Chehab,
Masahiro Yamada, Nayna Jain, Alexander Shishkin, Ingo Molnar,
Allison Randal, Jordan Niethe, Valentin Schneider, Arnd Bergmann,
Arnaldo Carvalho de Melo, Alexander Viro, Jonathan Cameron,
Namhyung Kim, Thomas Gleixner, Andy Shevchenko, Hari Bathini,
Greg Kroah-Hartman, Nicholas Piggin, Claudio Carvalho,
Eric Richter, Eric W. Biederman, linux-fsdevel, David S. Miller,
Thiago Jung Bauermann
In-Reply-To: <5cd926191175c4a4a85dc2246adc84bcfac21b1a.1584620202.git.msuchanek@suse.de>
On Thu, 2020-03-19 at 13:19 +0100, Michal Suchanek wrote:
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v10: new patch
> ---
> MAINTAINERS | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bc8dbe4fe4c9..329bf4a31412 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13088,6 +13088,8 @@ F: arch/*/kernel/*/perf_event*.c
> F: arch/*/kernel/*/*/perf_event*.c
> F: arch/*/include/asm/perf_event.h
> F: arch/*/kernel/perf_callchain.c
> +F: arch/*/perf/*
> +F: arch/*/perf/*/*
While I understand the desire, I believe that
repetitive listings like this don't really
help much.
Having a single entry of:
F: arch/*/perf/
would serve the same purpose.
Nominally, the difference between the 2 entries
vs the 1 entry is this:
F: arch/*/perf/*
Only the specific files in any directory that matches
this pattern but not any files in their subdirectories
are maintained.
F: arch/*/perf/*/*
Only the files in any top level subdirectory of any
directory that matches this pattern are maintained
but not files in any directory of those subdirectories.
F: arch/*/perf/
Any file or any file in any subdirectory of any
directory that matches this pattern is maintained.
^ permalink raw reply
* Re: [PATCH] tpm2: Export tpm2_get_cc_attrs_tbl for ibmvtpm driver as module
From: Stefan Berger @ 2020-03-19 15:56 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: sachinp, Stefan Berger, linux-kernel, linux-next, linux-integrity,
linuxppc-dev
In-Reply-To: <20200319142714.GB3703@linux.intel.com>
On 3/19/20 10:27 AM, Jarkko Sakkinen wrote:
> On Wed, Mar 18, 2020 at 03:53:54PM -0400, Stefan Berger wrote:
>> On 3/18/20 3:42 PM, Jarkko Sakkinen wrote:
>>> On Tue, Mar 17, 2020 at 09:08:19AM -0400, Stefan Berger wrote:
>>>> From: Stefan Berger <stefanb@linux.ibm.com>
>>>>
>>>> This patch fixes the following problem when the ibmvtpm driver
>>>> is built as a module:
>>>>
>>>> ERROR: modpost: "tpm2_get_cc_attrs_tbl" [drivers/char/tpm/tpm_ibmvtpm.ko] undefined!
>>>> make[1]: *** [scripts/Makefile.modpost:94: __modpost] Error 1
>>>> make: *** [Makefile:1298: modules] Error 2
>>>>
>>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>>> Hi, wrong tag (we use "tpm:"), missing fixes tag and please cc stable.
>>> Thanks.
>> I did not add the fixes tag because I do not know the final commit hash, or
>> is it the final commit hash once it is in linux-next? I doubt it with all
>> the merging that can occur.
> Can you send me a new version after rc1 is out?
Michael Ellerman (cc'ed) told me that the fixes tag should 'work' once
the bad patch is in linux-next. So I reposted yesterday (with a stray
'q' in the title :-( ):
https://lkml.org/lkml/2020/3/18/1283
Stefan
>
> /Jarkko
^ permalink raw reply
* Re: [patch V2 08/15] Documentation: Add lock ordering and nesting documentation
From: Jonathan Corbet @ 2020-03-19 15:04 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Oleg Nesterov, Joel Fernandes,
Will Deacon, Ingo Molnar, Davidlohr Bueso, Arnd Bergmann,
Logan Gunthorpe, Paul E . McKenney, linuxppc-dev, Steven Rostedt,
Bjorn Helgaas, Kurt Schwemmer, Kalle Valo, Felipe Balbi,
Greg Kroah-Hartman, linux-usb, linux-wireless, LKML, netdev,
Linus Torvalds, David S. Miller
In-Reply-To: <20200318204408.211530902@linutronix.de>
On Wed, 18 Mar 2020 21:43:10 +0100
Thomas Gleixner <tglx@linutronix.de> wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> The kernel provides a variety of locking primitives. The nesting of these
> lock types and the implications of them on RT enabled kernels is nowhere
> documented.
>
> Add initial documentation.
...time to add a a couple of nits...:)
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> V2: Addressed review comments from Randy
> ---
> Documentation/locking/index.rst | 1
> Documentation/locking/locktypes.rst | 298 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 299 insertions(+)
> create mode 100644 Documentation/locking/locktypes.rst
>
> --- a/Documentation/locking/index.rst
> +++ b/Documentation/locking/index.rst
> @@ -7,6 +7,7 @@ locking
> .. toctree::
> :maxdepth: 1
>
> + locktypes
> lockdep-design
> lockstat
> locktorture
> --- /dev/null
> +++ b/Documentation/locking/locktypes.rst
> @@ -0,0 +1,298 @@
> +.. _kernel_hacking_locktypes:
> +
So ... I vaguely remember that some Thomas guy added a document saying we
should be putting SPDX tags on our files? :)
> +==========================
> +Lock types and their rules
> +==========================
[...]
> +PREEMPT_RT caveats
> +==================
> +
> +spinlock_t and rwlock_t
> +-----------------------
> +
> +The substitution of spinlock_t and rwlock_t on PREEMPT_RT enabled kernels
> +with RT-mutex based implementations has a few implications.
> +
> +On a non PREEMPT_RT enabled kernel the following code construct is
> +perfectly fine::
> +
> + local_irq_disable();
> + spin_lock(&lock);
> +
> +and fully equivalent to::
> +
> + spin_lock_irq(&lock);
> +
> +Same applies to rwlock_t and the _irqsave() suffix variant.
> +
> +On a PREEMPT_RT enabled kernel this breaks because the RT-mutex
> +substitution expects a fully preemptible context.
> +
> +The preferred solution is to use :c:func:`spin_lock_irq()` or
> +:c:func:`spin_lock_irqsave()` and their unlock counterparts.
We don't need (and shouldn't use) :c:func: anymore; just saying
spin_lock_irq() will cause the Right Things to happen.
Thanks,
jon
^ permalink raw reply
* Re: [RFC 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Sachin Sant @ 2020-03-19 14:59 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Nathan Lynch, Srikar Dronamraju, Mel Gorman, Michal Hocko,
Pekka Enberg, linux-mm, Kirill Tkhai, David Rientjes,
Christopher Lameter, bharata, linuxppc-dev, Joonsoo Kim
In-Reply-To: <339cf655-393e-c48e-4797-86f61df56c35@suse.cz>
>>> Great, thanks! Can I add your Tested-by: then?
>>
>> Sure.
>> Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
>>
>> Thank you for the fix.
>
> Thanks! Sorry to bother, but in the end I decided to do further change so I
> would appreciate verification if it still works as intended.
Works as expected. I am able to boot the machine without any issues.
Thanks
-Sachin
^ permalink raw reply
* Re: [PATCH v11 4/8] powerpc/perf: consolidate valid_user_sp
From: Michal Suchánek @ 2020-03-19 14:46 UTC (permalink / raw)
To: Christophe Leroy
Cc: Mark Rutland, Gustavo Luiz Duarte, Peter Zijlstra,
Sebastian Andrzej Siewior, Linux Kernel Mailing List,
Paul Mackerras, Jiri Olsa, Rob Herring, Michael Neuling,
Mauro Carvalho Chehab, Masahiro Yamada, Nayna Jain,
linux-fsdevel @ vger . kernel . org --in-reply-to=,
Alexander Shishkin, Andy Shevchenko, Ingo Molnar, Allison Randal,
Jordan Niethe, Valentin Schneider, Arnd Bergmann,
Arnaldo Carvalho de Melo, Alexander Viro, Jonathan Cameron,
Namhyung Kim, Thomas Gleixner, Andy Shevchenko, Hari Bathini,
Greg Kroah-Hartman, Nicholas Piggin, Claudio Carvalho,
Eric Richter, Eric W. Biederman,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, David S. Miller,
Thiago Jung Bauermann
In-Reply-To: <8775f299-be1b-3457-c59d-e4f61d8223e5@c-s.fr>
On Thu, Mar 19, 2020 at 03:16:03PM +0100, Christophe Leroy wrote:
>
>
> Le 19/03/2020 à 14:35, Andy Shevchenko a écrit :
> > On Thu, Mar 19, 2020 at 1:54 PM Michal Suchanek <msuchanek@suse.de> wrote:
> > >
> > > Merge the 32bit and 64bit version.
> > >
> > > Halve the check constants on 32bit.
> > >
> > > Use STACK_TOP since it is defined.
> > >
> > > Passing is_64 is now redundant since is_32bit_task() is used to
> > > determine which callchain variant should be used. Use STACK_TOP and
> > > is_32bit_task() directly.
> > >
> > > This removes a page from the valid 32bit area on 64bit:
> > > #define TASK_SIZE_USER32 (0x0000000100000000UL - (1 * PAGE_SIZE))
> > > #define STACK_TOP_USER32 TASK_SIZE_USER32
> >
> > ...
> >
> > > +static inline int valid_user_sp(unsigned long sp)
> > > +{
> > > + bool is_64 = !is_32bit_task();
> > > +
> > > + if (!sp || (sp & (is_64 ? 7 : 3)) || sp > STACK_TOP - (is_64 ? 32 : 16))
> > > + return 0;
> > > + return 1;
> > > +}
> >
> > Other possibility:
>
> I prefer this one.
>
> >
> > unsigned long align = is_32bit_task() ? 3 : 7;
>
> I would call it mask instead of align
>
> > unsigned long top = STACK_TOP - (is_32bit_task() ? 16 : 32);
> >
> > return !(!sp || (sp & align) || sp > top);
And we can avoid the inversion here as well as in !valid_user_sp(sp) by
changing to invalid_user_sp.
Thanks
Michal
^ permalink raw reply
* Re: [PATCH] tpm2: Export tpm2_get_cc_attrs_tbl for ibmvtpm driver as module
From: Jarkko Sakkinen @ 2020-03-19 14:27 UTC (permalink / raw)
To: Stefan Berger
Cc: sachinp, Stefan Berger, linux-kernel, linux-next, linux-integrity,
linuxppc-dev
In-Reply-To: <4b2949a9-b251-906d-d513-1b2ccef758a0@linux.ibm.com>
On Wed, Mar 18, 2020 at 03:53:54PM -0400, Stefan Berger wrote:
> On 3/18/20 3:42 PM, Jarkko Sakkinen wrote:
> > On Tue, Mar 17, 2020 at 09:08:19AM -0400, Stefan Berger wrote:
> > > From: Stefan Berger <stefanb@linux.ibm.com>
> > >
> > > This patch fixes the following problem when the ibmvtpm driver
> > > is built as a module:
> > >
> > > ERROR: modpost: "tpm2_get_cc_attrs_tbl" [drivers/char/tpm/tpm_ibmvtpm.ko] undefined!
> > > make[1]: *** [scripts/Makefile.modpost:94: __modpost] Error 1
> > > make: *** [Makefile:1298: modules] Error 2
> > >
> > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > Hi, wrong tag (we use "tpm:"), missing fixes tag and please cc stable.
> > Thanks.
>
> I did not add the fixes tag because I do not know the final commit hash, or
> is it the final commit hash once it is in linux-next? I doubt it with all
> the merging that can occur.
Can you send me a new version after rc1 is out?
/Jarkko
^ permalink raw reply
* Re: [PATCH v11 8/8] MAINTAINERS: perf: Add pattern that matches ppc perf to the perf entry.
From: Andy Shevchenko @ 2020-03-19 14:26 UTC (permalink / raw)
To: Michal Suchánek
Cc: Mark Rutland, Gustavo Luiz Duarte, Peter Zijlstra,
Sebastian Andrzej Siewior, Linux Kernel Mailing List,
Paul Mackerras, Jiri Olsa, Rob Herring, Michael Neuling,
Mauro Carvalho Chehab, Masahiro Yamada, Nayna Jain,
Alexander Shishkin, Ingo Molnar, Allison Randal, Jordan Niethe,
Valentin Schneider, Arnd Bergmann, Arnaldo Carvalho de Melo,
Alexander Viro, Jonathan Cameron, Namhyung Kim, Thomas Gleixner,
Hari Bathini, Greg Kroah-Hartman, Nicholas Piggin,
Claudio Carvalho, Eric Richter, Eric W. Biederman, Linux FS Devel,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, David S. Miller,
Thiago Jung Bauermann
In-Reply-To: <20200319140008.GI25468@kitsune.suse.cz>
On Thu, Mar 19, 2020 at 03:00:08PM +0100, Michal Suchánek wrote:
> On Thu, Mar 19, 2020 at 03:37:03PM +0200, Andy Shevchenko wrote:
> > On Thu, Mar 19, 2020 at 2:21 PM Michal Suchanek <msuchanek@suse.de> wrote:
> > >
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > > v10: new patch
> > > ---
> > > MAINTAINERS | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index bc8dbe4fe4c9..329bf4a31412 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -13088,6 +13088,8 @@ F: arch/*/kernel/*/perf_event*.c
> > > F: arch/*/kernel/*/*/perf_event*.c
> > > F: arch/*/include/asm/perf_event.h
> > > F: arch/*/kernel/perf_callchain.c
> > > +F: arch/*/perf/*
> > > +F: arch/*/perf/*/*
> > > F: arch/*/events/*
> > > F: arch/*/events/*/*
> > > F: tools/perf/
> >
> > Had you run parse-maintainers.pl?
> Did not know it exists. The output is:
>
> scripts/parse-maintainers.pl
> Odd non-pattern line '
> Documentation/devicetree/bindings/media/ti,cal.yaml
> ' for 'TI VPE/CAL DRIVERS' at scripts/parse-maintainers.pl line 147,
> <$file> line 16756.
It is fixed in media tree and available in linux next as
d44535cb14c9 ("media: MAINTAINERS: Sort entries in database for TI VPE/CAL")
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v11 4/8] powerpc/perf: consolidate valid_user_sp
From: Christophe Leroy @ 2020-03-19 14:16 UTC (permalink / raw)
To: Andy Shevchenko, Michal Suchanek
Cc: Mark Rutland, Gustavo Luiz Duarte, Peter Zijlstra,
Sebastian Andrzej Siewior, Linux Kernel Mailing List,
Paul Mackerras, Jiri Olsa, Rob Herring, Michael Neuling,
Mauro Carvalho Chehab, Masahiro Yamada, Nayna Jain,
linux-fsdevel @ vger . kernel . org --in-reply-to=,
Alexander Shishkin, Ingo Molnar, Allison Randal, Jordan Niethe,
Valentin Schneider, Arnd Bergmann, Arnaldo Carvalho de Melo,
Alexander Viro, Jonathan Cameron, Namhyung Kim, Thomas Gleixner,
Andy Shevchenko, Hari Bathini, Greg Kroah-Hartman,
Nicholas Piggin, Claudio Carvalho, Eric Richter,
Eric W. Biederman, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
David S. Miller, Thiago Jung Bauermann
In-Reply-To: <CAHp75VcMkPeJ6exroipnxvf-7g-C8QbVm0bAnp=rk505_nxySw@mail.gmail.com>
Le 19/03/2020 à 14:35, Andy Shevchenko a écrit :
> On Thu, Mar 19, 2020 at 1:54 PM Michal Suchanek <msuchanek@suse.de> wrote:
>>
>> Merge the 32bit and 64bit version.
>>
>> Halve the check constants on 32bit.
>>
>> Use STACK_TOP since it is defined.
>>
>> Passing is_64 is now redundant since is_32bit_task() is used to
>> determine which callchain variant should be used. Use STACK_TOP and
>> is_32bit_task() directly.
>>
>> This removes a page from the valid 32bit area on 64bit:
>> #define TASK_SIZE_USER32 (0x0000000100000000UL - (1 * PAGE_SIZE))
>> #define STACK_TOP_USER32 TASK_SIZE_USER32
>
> ...
>
>> +static inline int valid_user_sp(unsigned long sp)
>> +{
>> + bool is_64 = !is_32bit_task();
>> +
>> + if (!sp || (sp & (is_64 ? 7 : 3)) || sp > STACK_TOP - (is_64 ? 32 : 16))
>> + return 0;
>> + return 1;
>> +}
>
> Perhaps better to read
>
> if (!sp)
> return 0;
>
> if (is_32bit_task()) {
> if (sp & 0x03)
> return 0;
> if (sp > STACK_TOP - 16)
> return 0;
> } else {
> ...
> }
>
> return 1;
>
> Other possibility:
I prefer this one.
>
> unsigned long align = is_32bit_task() ? 3 : 7;
I would call it mask instead of align
> unsigned long top = STACK_TOP - (is_32bit_task() ? 16 : 32);
>
> return !(!sp || (sp & align) || sp > top);
>
Christophe
^ permalink raw reply
* Re: [RFC 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Vlastimil Babka @ 2020-03-19 14:10 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Sachin Sant, Nathan Lynch, Mel Gorman, Michal Hocko, Pekka Enberg,
linux-mm, Kirill Tkhai, David Rientjes, Christopher Lameter,
bharata, linuxppc-dev, Joonsoo Kim
In-Reply-To: <20200319140549.GF4879@linux.vnet.ibm.com>
On 3/19/20 3:05 PM, Srikar Dronamraju wrote:
> * Vlastimil Babka <vbabka@suse.cz> [2020-03-19 14:47:58]:
>
>> ----8<----
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 17dc00e33115..7113b1f9cd77 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -1973,8 +1973,6 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
>>
>> if (node == NUMA_NO_NODE)
>> searchnode = numa_mem_id();
>> - else if (!node_present_pages(node))
>> - searchnode = node_to_mem_node(node);
>>
>> object = get_partial_node(s, get_node(s, searchnode), c, flags);
>
> Are we okay with passing a node to get_partial_node with !NUMA_NO_NODE and
> !N_MEMORY including possible nodes?
No, but AFAICS, such node values are already handled in ___slab_alloc, and
cannot reach get_partial(). If you see something I missed, please do tell.
>> if (object || node != NUMA_NO_NODE)
>
^ permalink raw reply
* Re: [RFC 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Srikar Dronamraju @ 2020-03-19 14:05 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Sachin Sant, Nathan Lynch, Mel Gorman, Michal Hocko, Pekka Enberg,
linux-mm, Kirill Tkhai, David Rientjes, Christopher Lameter,
bharata, linuxppc-dev, Joonsoo Kim
In-Reply-To: <339cf655-393e-c48e-4797-86f61df56c35@suse.cz>
* Vlastimil Babka <vbabka@suse.cz> [2020-03-19 14:47:58]:
> ----8<----
> diff --git a/mm/slub.c b/mm/slub.c
> index 17dc00e33115..7113b1f9cd77 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1973,8 +1973,6 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
>
> if (node == NUMA_NO_NODE)
> searchnode = numa_mem_id();
> - else if (!node_present_pages(node))
> - searchnode = node_to_mem_node(node);
>
> object = get_partial_node(s, get_node(s, searchnode), c, flags);
Are we okay with passing a node to get_partial_node with !NUMA_NO_NODE and
!N_MEMORY including possible nodes?
> if (object || node != NUMA_NO_NODE)
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH v11 4/8] powerpc/perf: consolidate valid_user_sp
From: Michal Suchánek @ 2020-03-19 14:04 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mark Rutland, Gustavo Luiz Duarte, Peter Zijlstra,
Sebastian Andrzej Siewior, Linux Kernel Mailing List,
Paul Mackerras, Jiri Olsa, Rob Herring, Michael Neuling,
Mauro Carvalho Chehab, Masahiro Yamada, Nayna Jain,
linux-fsdevel @ vger . kernel . org --in-reply-to=,
Alexander Shishkin, Ingo Molnar, Allison Randal, Jordan Niethe,
Valentin Schneider, Arnd Bergmann, Arnaldo Carvalho de Melo,
Alexander Viro, Jonathan Cameron, Namhyung Kim, Thomas Gleixner,
Andy Shevchenko, Hari Bathini, Greg Kroah-Hartman,
Nicholas Piggin, Claudio Carvalho, Eric Richter,
Eric W. Biederman, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
David S. Miller, Thiago Jung Bauermann
In-Reply-To: <CAHp75VcMkPeJ6exroipnxvf-7g-C8QbVm0bAnp=rk505_nxySw@mail.gmail.com>
On Thu, Mar 19, 2020 at 03:35:03PM +0200, Andy Shevchenko wrote:
> On Thu, Mar 19, 2020 at 1:54 PM Michal Suchanek <msuchanek@suse.de> wrote:
> >
> > Merge the 32bit and 64bit version.
> >
> > Halve the check constants on 32bit.
> >
> > Use STACK_TOP since it is defined.
> >
> > Passing is_64 is now redundant since is_32bit_task() is used to
> > determine which callchain variant should be used. Use STACK_TOP and
> > is_32bit_task() directly.
> >
> > This removes a page from the valid 32bit area on 64bit:
> > #define TASK_SIZE_USER32 (0x0000000100000000UL - (1 * PAGE_SIZE))
> > #define STACK_TOP_USER32 TASK_SIZE_USER32
>
> ...
>
> > +static inline int valid_user_sp(unsigned long sp)
> > +{
> > + bool is_64 = !is_32bit_task();
> > +
> > + if (!sp || (sp & (is_64 ? 7 : 3)) || sp > STACK_TOP - (is_64 ? 32 : 16))
> > + return 0;
> > + return 1;
> > +}
>
> Perhaps better to read
>
> if (!sp)
> return 0;
>
> if (is_32bit_task()) {
> if (sp & 0x03)
> return 0;
> if (sp > STACK_TOP - 16)
> return 0;
> } else {
> ...
> }
>
> return 1;
>
> Other possibility:
>
> unsigned long align = is_32bit_task() ? 3 : 7;
> unsigned long top = STACK_TOP - (is_32bit_task() ? 16 : 32);
>
> return !(!sp || (sp & align) || sp > top);
Sounds reasonale.
Thanks
Michal
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v11 0/8] Disable compat cruft on ppc64le v11
From: Michal Suchánek @ 2020-03-19 14:01 UTC (permalink / raw)
To: Christophe Leroy
Cc: Mark Rutland, Gustavo Luiz Duarte, Peter Zijlstra,
Sebastian Andrzej Siewior, linux-kernel, Paul Mackerras,
Jiri Olsa, Rob Herring, Michael Neuling, Mauro Carvalho Chehab,
Masahiro Yamada, Nayna Jain, Alexander Shishkin, Ingo Molnar,
Allison Randal, Jordan Niethe, Valentin Schneider, Arnd Bergmann,
Arnaldo Carvalho de Melo, Alexander Viro, Jonathan Cameron,
Namhyung Kim, Thomas Gleixner, Andy Shevchenko, Hari Bathini,
Greg Kroah-Hartman, Nicholas Piggin, Claudio Carvalho,
Eric Richter, Eric W. Biederman, linux-fsdevel, linuxppc-dev,
David S. Miller, Thiago Jung Bauermann
In-Reply-To: <c5f30914-4d0c-5437-e8c4-9d62d08061e1@c-s.fr>
On Thu, Mar 19, 2020 at 01:36:56PM +0100, Christophe Leroy wrote:
> You sent it twice ? Any difference between the two dispatch ?
Some headers were broken the first time around.
Thanks
Michal
>
> Christophe
>
> Le 19/03/2020 à 13:19, Michal Suchanek a écrit :
> > Less code means less bugs so add a knob to skip the compat stuff.
> >
> > Changes in v2: saner CONFIG_COMPAT ifdefs
> > Changes in v3:
> > - change llseek to 32bit instead of builing it unconditionally in fs
> > - clanup the makefile conditionals
> > - remove some ifdefs or convert to IS_DEFINED where possible
> > Changes in v4:
> > - cleanup is_32bit_task and current_is_64bit
> > - more makefile cleanup
> > Changes in v5:
> > - more current_is_64bit cleanup
> > - split off callchain.c 32bit and 64bit parts
> > Changes in v6:
> > - cleanup makefile after split
> > - consolidate read_user_stack_32
> > - fix some checkpatch warnings
> > Changes in v7:
> > - add back __ARCH_WANT_SYS_LLSEEK to fix build with llseek
> > - remove leftover hunk
> > - add review tags
> > Changes in v8:
> > - consolidate valid_user_sp to fix it in the split callchain.c
> > - fix build errors/warnings with PPC64 !COMPAT and PPC32
> > Changes in v9:
> > - remove current_is_64bit()
> > Chanegs in v10:
> > - rebase, sent together with the syscall cleanup
> > Changes in v11:
> > - rebase
> > - add MAINTAINERS pattern for ppc perf
> >
> > Michal Suchanek (8):
> > powerpc: Add back __ARCH_WANT_SYS_LLSEEK macro
> > powerpc: move common register copy functions from signal_32.c to
> > signal.c
> > powerpc/perf: consolidate read_user_stack_32
> > powerpc/perf: consolidate valid_user_sp
> > powerpc/64: make buildable without CONFIG_COMPAT
> > powerpc/64: Make COMPAT user-selectable disabled on littleendian by
> > default.
> > powerpc/perf: split callchain.c by bitness
> > MAINTAINERS: perf: Add pattern that matches ppc perf to the perf
> > entry.
> >
> > MAINTAINERS | 2 +
> > arch/powerpc/Kconfig | 5 +-
> > arch/powerpc/include/asm/thread_info.h | 4 +-
> > arch/powerpc/include/asm/unistd.h | 1 +
> > arch/powerpc/kernel/Makefile | 6 +-
> > arch/powerpc/kernel/entry_64.S | 2 +
> > arch/powerpc/kernel/signal.c | 144 +++++++++-
> > arch/powerpc/kernel/signal_32.c | 140 ----------
> > arch/powerpc/kernel/syscall_64.c | 6 +-
> > arch/powerpc/kernel/vdso.c | 3 +-
> > arch/powerpc/perf/Makefile | 5 +-
> > arch/powerpc/perf/callchain.c | 356 +------------------------
> > arch/powerpc/perf/callchain.h | 20 ++
> > arch/powerpc/perf/callchain_32.c | 196 ++++++++++++++
> > arch/powerpc/perf/callchain_64.c | 174 ++++++++++++
> > fs/read_write.c | 3 +-
> > 16 files changed, 556 insertions(+), 511 deletions(-)
> > create mode 100644 arch/powerpc/perf/callchain.h
> > create mode 100644 arch/powerpc/perf/callchain_32.c
> > create mode 100644 arch/powerpc/perf/callchain_64.c
> >
^ permalink raw reply
* Re: [PATCH v11 8/8] MAINTAINERS: perf: Add pattern that matches ppc perf to the perf entry.
From: Michal Suchánek @ 2020-03-19 14:00 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mark Rutland, Gustavo Luiz Duarte, Peter Zijlstra,
Sebastian Andrzej Siewior, Linux Kernel Mailing List,
Paul Mackerras, Jiri Olsa, Rob Herring, Michael Neuling,
Mauro Carvalho Chehab, Masahiro Yamada, Nayna Jain,
Alexander Shishkin, Ingo Molnar, Allison Randal, Jordan Niethe,
Valentin Schneider, Arnd Bergmann, Arnaldo Carvalho de Melo,
Alexander Viro, Jonathan Cameron, Namhyung Kim, Thomas Gleixner,
Andy Shevchenko, Hari Bathini, Greg Kroah-Hartman,
Nicholas Piggin, Claudio Carvalho, Eric Richter,
Eric W. Biederman, Linux FS Devel,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, David S. Miller,
Thiago Jung Bauermann
In-Reply-To: <CAHp75VegYhz-hwSUNHbGFB3yiatAWWytwB7Vctf=mCLyCJEy3Q@mail.gmail.com>
On Thu, Mar 19, 2020 at 03:37:03PM +0200, Andy Shevchenko wrote:
> On Thu, Mar 19, 2020 at 2:21 PM Michal Suchanek <msuchanek@suse.de> wrote:
> >
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > v10: new patch
> > ---
> > MAINTAINERS | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index bc8dbe4fe4c9..329bf4a31412 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -13088,6 +13088,8 @@ F: arch/*/kernel/*/perf_event*.c
> > F: arch/*/kernel/*/*/perf_event*.c
> > F: arch/*/include/asm/perf_event.h
> > F: arch/*/kernel/perf_callchain.c
> > +F: arch/*/perf/*
> > +F: arch/*/perf/*/*
> > F: arch/*/events/*
> > F: arch/*/events/*/*
> > F: tools/perf/
>
> Had you run parse-maintainers.pl?
Did not know it exists. The output is:
scripts/parse-maintainers.pl
Odd non-pattern line '
Documentation/devicetree/bindings/media/ti,cal.yaml
' for 'TI VPE/CAL DRIVERS' at scripts/parse-maintainers.pl line 147,
<$file> line 16756.
Thanks
Michal
^ permalink raw reply
* Re: [RFC 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Vlastimil Babka @ 2020-03-19 13:47 UTC (permalink / raw)
To: Sachin Sant
Cc: Nathan Lynch, Srikar Dronamraju, Mel Gorman, Michal Hocko,
Pekka Enberg, linux-mm, Kirill Tkhai, David Rientjes,
Christopher Lameter, bharata, linuxppc-dev, Joonsoo Kim
In-Reply-To: <658E6AB8-581F-4722-BCBB-4BDD2245D265@linux.vnet.ibm.com>
On 3/19/20 2:26 PM, Sachin Sant wrote:
>
>
>> On 19-Mar-2020, at 6:53 PM, Vlastimil Babka <vbabka@suse.cz> wrote:
>>
>> On 3/19/20 9:52 AM, Sachin Sant wrote:
>>>
>>>> OK how about this version? It's somewhat ugly, but important is that the fast
>>>> path case (c->page exists) is unaffected and another common case (c->page is
>>>> NULL, but node is NUMA_NO_NODE) is just one extra check - impossible to avoid at
>>>> some point anyway.
>>>>
>>>
>>> I attempted the suggested tests.
>>>
>>> Test 1: March 18 linux-next + Patch 1 [1] + Patch 2 [2]
>>>
>>> Machine boots fine. numactl o/p after boot:
>>
>> Great, thanks! Can I add your Tested-by: then?
>
> Sure.
> Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
>
> Thank you for the fix.
Thanks! Sorry to bother, but in the end I decided to do further change so I
would appreciate verification if it still works as intended.
The point is to use node_state(N_NORMAL_MEMORY) instead of node_present_pages(),
as that is really what SLUB uses to decide whether to allocate the
kmem_cache_node. So we should match this properly given the opportunity.
I have also again removed the node_online() check in alloc_slab_page() as it
really shouldn't be reachable with an offline node - everything is taken care of
in ___slab_alloc, or callers use NUMA_NO_NODE.
----8<----
diff --git a/mm/slub.c b/mm/slub.c
index 17dc00e33115..7113b1f9cd77 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1973,8 +1973,6 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2563,17 +2561,27 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
struct page *page;
page = c->page;
- if (!page)
+ if (!page) {
+ /*
+ * if the node is not online or has no normal memory, just
+ * ignore the node constraint
+ */
+ if (unlikely(node != NUMA_NO_NODE &&
+ !node_state(node, N_NORMAL_MEMORY)))
+ node = NUMA_NO_NODE;
goto new_slab;
+ }
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * same as above but node_match() being false already
+ * implies node != NUMA_NO_NODE
+ */
+ if (!node_state(node, N_NORMAL_MEMORY)) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist, c);
goto new_slab;
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v11 8/8] MAINTAINERS: perf: Add pattern that matches ppc perf to the perf entry.
From: Andy Shevchenko @ 2020-03-19 13:37 UTC (permalink / raw)
To: Michal Suchanek
Cc: Mark Rutland, Gustavo Luiz Duarte, Peter Zijlstra,
Sebastian Andrzej Siewior, Linux Kernel Mailing List,
Paul Mackerras, Jiri Olsa, Rob Herring, Michael Neuling,
Mauro Carvalho Chehab, Masahiro Yamada, Nayna Jain,
Alexander Shishkin, Ingo Molnar, Allison Randal, Jordan Niethe,
Valentin Schneider, Arnd Bergmann, Arnaldo Carvalho de Melo,
Alexander Viro, Jonathan Cameron, Namhyung Kim, Thomas Gleixner,
Andy Shevchenko, Hari Bathini, Greg Kroah-Hartman,
Nicholas Piggin, Claudio Carvalho, Eric Richter,
Eric W. Biederman, Linux FS Devel,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, David S. Miller,
Thiago Jung Bauermann
In-Reply-To: <5cd926191175c4a4a85dc2246adc84bcfac21b1a.1584620202.git.msuchanek@suse.de>
On Thu, Mar 19, 2020 at 2:21 PM Michal Suchanek <msuchanek@suse.de> wrote:
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v10: new patch
> ---
> MAINTAINERS | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bc8dbe4fe4c9..329bf4a31412 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13088,6 +13088,8 @@ F: arch/*/kernel/*/perf_event*.c
> F: arch/*/kernel/*/*/perf_event*.c
> F: arch/*/include/asm/perf_event.h
> F: arch/*/kernel/perf_callchain.c
> +F: arch/*/perf/*
> +F: arch/*/perf/*/*
> F: arch/*/events/*
> F: arch/*/events/*/*
> F: tools/perf/
Had you run parse-maintainers.pl?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v11 4/8] powerpc/perf: consolidate valid_user_sp
From: Andy Shevchenko @ 2020-03-19 13:35 UTC (permalink / raw)
To: Michal Suchanek
Cc: Mark Rutland, Gustavo Luiz Duarte, Peter Zijlstra,
Sebastian Andrzej Siewior, Linux Kernel Mailing List,
Paul Mackerras, Jiri Olsa, Rob Herring, Michael Neuling,
Mauro Carvalho Chehab, Masahiro Yamada, Nayna Jain,
linux-fsdevel @ vger . kernel . org --in-reply-to=,
Alexander Shishkin, Ingo Molnar, Allison Randal, Jordan Niethe,
Valentin Schneider, Arnd Bergmann, Arnaldo Carvalho de Melo,
Alexander Viro, Jonathan Cameron, Namhyung Kim, Thomas Gleixner,
Andy Shevchenko, Hari Bathini, Greg Kroah-Hartman,
Nicholas Piggin, Claudio Carvalho, Eric Richter,
Eric W. Biederman, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
David S. Miller, Thiago Jung Bauermann
In-Reply-To: <1b612025371bb9f2bcce72c700c809ae29e57392.1584613649.git.msuchanek@suse.de>
On Thu, Mar 19, 2020 at 1:54 PM Michal Suchanek <msuchanek@suse.de> wrote:
>
> Merge the 32bit and 64bit version.
>
> Halve the check constants on 32bit.
>
> Use STACK_TOP since it is defined.
>
> Passing is_64 is now redundant since is_32bit_task() is used to
> determine which callchain variant should be used. Use STACK_TOP and
> is_32bit_task() directly.
>
> This removes a page from the valid 32bit area on 64bit:
> #define TASK_SIZE_USER32 (0x0000000100000000UL - (1 * PAGE_SIZE))
> #define STACK_TOP_USER32 TASK_SIZE_USER32
...
> +static inline int valid_user_sp(unsigned long sp)
> +{
> + bool is_64 = !is_32bit_task();
> +
> + if (!sp || (sp & (is_64 ? 7 : 3)) || sp > STACK_TOP - (is_64 ? 32 : 16))
> + return 0;
> + return 1;
> +}
Perhaps better to read
if (!sp)
return 0;
if (is_32bit_task()) {
if (sp & 0x03)
return 0;
if (sp > STACK_TOP - 16)
return 0;
} else {
...
}
return 1;
Other possibility:
unsigned long align = is_32bit_task() ? 3 : 7;
unsigned long top = STACK_TOP - (is_32bit_task() ? 16 : 32);
return !(!sp || (sp & align) || sp > top);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [RFC 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Sachin Sant @ 2020-03-19 13:26 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Nathan Lynch, Srikar Dronamraju, Mel Gorman, Michal Hocko,
Pekka Enberg, linux-mm, Kirill Tkhai, David Rientjes,
Christopher Lameter, bharata, linuxppc-dev, Joonsoo Kim
In-Reply-To: <b9b95895-ca6b-5ad2-1f67-45fee93d1e67@suse.cz>
> On 19-Mar-2020, at 6:53 PM, Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 3/19/20 9:52 AM, Sachin Sant wrote:
>>
>>> OK how about this version? It's somewhat ugly, but important is that the fast
>>> path case (c->page exists) is unaffected and another common case (c->page is
>>> NULL, but node is NUMA_NO_NODE) is just one extra check - impossible to avoid at
>>> some point anyway.
>>>
>>
>> I attempted the suggested tests.
>>
>> Test 1: March 18 linux-next + Patch 1 [1] + Patch 2 [2]
>>
>> Machine boots fine. numactl o/p after boot:
>
> Great, thanks! Can I add your Tested-by: then?
Sure.
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Thank you for the fix.
Thanks
-Sachin
^ permalink raw reply
* Re: [RFC 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Vlastimil Babka @ 2020-03-19 13:23 UTC (permalink / raw)
To: Sachin Sant
Cc: Nathan Lynch, Srikar Dronamraju, Mel Gorman, Michal Hocko,
Pekka Enberg, linux-mm, Kirill Tkhai, David Rientjes,
Christopher Lameter, bharata, linuxppc-dev, Joonsoo Kim
In-Reply-To: <0F67B5AA-96DF-4977-BDC6-D72959B3F7EF@linux.vnet.ibm.com>
On 3/19/20 9:52 AM, Sachin Sant wrote:
>
>> OK how about this version? It's somewhat ugly, but important is that the fast
>> path case (c->page exists) is unaffected and another common case (c->page is
>> NULL, but node is NUMA_NO_NODE) is just one extra check - impossible to avoid at
>> some point anyway.
>>
>
> I attempted the suggested tests.
>
> Test 1: March 18 linux-next + Patch 1 [1] + Patch 2 [2]
>
> Machine boots fine. numactl o/p after boot:
Great, thanks! Can I add your Tested-by: then?
If Bharata confirms his scenario with updated patch, I will send it properly.
> # numactl -H
> available: 2 nodes (0-1)
> node 0 cpus:
> node 0 size: 0 MB
> node 0 free: 0 MB
> node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
> node 1 size: 35631 MB
> node 1 free: 32724 MB
> node distances:
> node 0 1
> 0: 10 40
> 1: 40 10
> #
>
> Test 2: Code base as used in Test 1 + 3 patch series from Srikar [3]
>
> Machine boots fine. numactl o/p after boot:
>
> # numactl -H
> available: 1 nodes (1)
> node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
> node 1 size: 35631 MB
> node 1 free: 32711 MB
> node distances:
> node 1
> 1: 10
> #
>
> Thanks!
> -Sachin
>
>
> [1] https://lore.kernel.org/linux-mm/e060ad43-ff4e-0e59-2e64-ce8a4916ec70@suse.cz/T/#mb8d0a358dc5c5fb9661fa45047072a92c510faf2
> [2] https://lore.kernel.org/linux-mm/e060ad43-ff4e-0e59-2e64-ce8a4916ec70@suse.cz/T/#mce342e54a95ce2ee3043901e52d70ee2fd94c516
> [3] https://lore.kernel.org/linux-mm/20200311110237.5731-1-srikar@linux.vnet.ibm.com/
>
^ permalink raw reply
* [PATCH v3 8/8] mm/memory_hotplug: allow to specify a default online_type
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-hyperv, Michal Hocko, Baoquan He, David Hildenbrand,
Greg Kroah-Hartman, Rafael J. Wysocki, Wei Yang, linux-mm,
Andrew Morton, Michal Hocko, linuxppc-dev, Oscar Salvador
In-Reply-To: <20200319131221.14044-1-david@redhat.com>
For now, distributions implement advanced udev rules to essentially
- Don't online any hotplugged memory (s390x)
- Online all memory to ZONE_NORMAL (e.g., most virt environments like
hyperv)
- Online all memory to ZONE_MOVABLE in case the zone imbalance is taken
care of (e.g., bare metal, special virt environments)
In summary: All memory is usually onlined the same way, however, the
kernel always has to ask user space to come up with the same answer.
E.g., Hyper-V always waits for a memory block to get onlined before
continuing, otherwise it might end up adding memory faster than
onlining it, which can result in strange OOM situations. This waiting
slows down adding of a bigger amount of memory.
Let's allow to specify a default online_type, not just "online" and
"offline". This allows distributions to configure the default online_type
when booting up and be done with it.
We can now specify "offline", "online", "online_movable" and
"online_kernel" via
- "memhp_default_state=" on the kernel cmdline
- /sys/devices/system/memory/auto_online_blocks
just like we are able to specify for a single memory block via
/sys/devices/system/memory/memoryX/state
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/base/memory.c | 11 +++++------
include/linux/memory_hotplug.h | 2 ++
mm/memory_hotplug.c | 8 ++++----
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 8d3e16dab69f..2b09b68b9f78 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -35,7 +35,7 @@ static const char *const online_type_to_str[] = {
[MMOP_ONLINE_MOVABLE] = "online_movable",
};
-static int memhp_online_type_from_str(const char *str)
+int memhp_online_type_from_str(const char *str)
{
int i;
@@ -394,13 +394,12 @@ static ssize_t auto_online_blocks_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
- if (sysfs_streq(buf, "online"))
- memhp_default_online_type = MMOP_ONLINE;
- else if (sysfs_streq(buf, "offline"))
- memhp_default_online_type = MMOP_OFFLINE;
- else
+ const int online_type = memhp_online_type_from_str(buf);
+
+ if (online_type < 0)
return -EINVAL;
+ memhp_default_online_type = online_type;
return count;
}
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 6d6f85bb66e9..93d9ada74ddd 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -118,6 +118,8 @@ extern int arch_add_memory(int nid, u64 start, u64 size,
struct mhp_params *params);
extern u64 max_mem_size;
+extern int memhp_online_type_from_str(const char *str);
+
/* Default online_type (MMOP_*) when new memory blocks are added. */
extern int memhp_default_online_type;
/* If movable_node boot option specified */
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 4efcf8cb9ac5..89197163d138 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -74,10 +74,10 @@ int memhp_default_online_type = MMOP_ONLINE;
static int __init setup_memhp_default_state(char *str)
{
- if (!strcmp(str, "online"))
- memhp_default_online_type = MMOP_ONLINE;
- else if (!strcmp(str, "offline"))
- memhp_default_online_type = MMOP_OFFLINE;
+ const int online_type = memhp_online_type_from_str(str);
+
+ if (online_type >= 0)
+ memhp_default_online_type = online_type;
return 1;
}
--
2.24.1
^ permalink raw reply related
* [PATCH v3 7/8] mm/memory_hotplug: convert memhp_auto_online to store an online_type
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-hyperv, Michal Hocko, Baoquan He, David Hildenbrand,
Greg Kroah-Hartman, Rafael J. Wysocki, Wei Yang, linux-mm,
Andrew Morton, Michal Hocko, linuxppc-dev, Oscar Salvador
In-Reply-To: <20200319131221.14044-1-david@redhat.com>
... and rename it to memhp_default_online_type. This is a preparation
for more detailed default online behavior.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/base/memory.c | 10 ++++------
include/linux/memory_hotplug.h | 3 ++-
mm/memory_hotplug.c | 11 ++++++-----
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 8a7f29c0bf97..8d3e16dab69f 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -386,10 +386,8 @@ static DEVICE_ATTR_RO(block_size_bytes);
static ssize_t auto_online_blocks_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- if (memhp_auto_online)
- return sprintf(buf, "online\n");
- else
- return sprintf(buf, "offline\n");
+ return sprintf(buf, "%s\n",
+ online_type_to_str[memhp_default_online_type]);
}
static ssize_t auto_online_blocks_store(struct device *dev,
@@ -397,9 +395,9 @@ static ssize_t auto_online_blocks_store(struct device *dev,
const char *buf, size_t count)
{
if (sysfs_streq(buf, "online"))
- memhp_auto_online = true;
+ memhp_default_online_type = MMOP_ONLINE;
else if (sysfs_streq(buf, "offline"))
- memhp_auto_online = false;
+ memhp_default_online_type = MMOP_OFFLINE;
else
return -EINVAL;
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 76f3c617a8ab..6d6f85bb66e9 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -118,7 +118,8 @@ extern int arch_add_memory(int nid, u64 start, u64 size,
struct mhp_params *params);
extern u64 max_mem_size;
-extern bool memhp_auto_online;
+/* Default online_type (MMOP_*) when new memory blocks are added. */
+extern int memhp_default_online_type;
/* If movable_node boot option specified */
extern bool movable_node_enabled;
static inline bool movable_node_is_enabled(void)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index e21a7d53ade5..4efcf8cb9ac5 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -67,17 +67,17 @@ void put_online_mems(void)
bool movable_node_enabled = false;
#ifndef CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE
-bool memhp_auto_online;
+int memhp_default_online_type = MMOP_OFFLINE;
#else
-bool memhp_auto_online = true;
+int memhp_default_online_type = MMOP_ONLINE;
#endif
static int __init setup_memhp_default_state(char *str)
{
if (!strcmp(str, "online"))
- memhp_auto_online = true;
+ memhp_default_online_type = MMOP_ONLINE;
else if (!strcmp(str, "offline"))
- memhp_auto_online = false;
+ memhp_default_online_type = MMOP_OFFLINE;
return 1;
}
@@ -993,6 +993,7 @@ static int check_hotplug_memory_range(u64 start, u64 size)
static int online_memory_block(struct memory_block *mem, void *arg)
{
+ mem->online_type = memhp_default_online_type;
return device_online(&mem->dev);
}
@@ -1065,7 +1066,7 @@ int __ref add_memory_resource(int nid, struct resource *res)
mem_hotplug_done();
/* online pages if requested */
- if (memhp_auto_online)
+ if (memhp_default_online_type != MMOP_OFFLINE)
walk_memory_blocks(start, size, NULL, online_memory_block);
return ret;
--
2.24.1
^ permalink raw reply related
* [PATCH v3 5/8] hv_balloon: don't check for memhp_auto_online manually
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-hyperv, Stephen Hemminger, Baoquan He, David Hildenbrand,
Haiyang Zhang, Rafael J. Wysocki, Wei Liu, Michal Hocko, linux-mm,
Wei Yang, Vitaly Kuznetsov, K. Y. Srinivasan, linuxppc-dev,
Andrew Morton, Oscar Salvador
In-Reply-To: <20200319131221.14044-1-david@redhat.com>
We get the MEM_ONLINE notifier call if memory is added right from the
kernel via add_memory() or later from user space.
Let's get rid of the "ha_waiting" flag - the wait event has an inbuilt
mechanism (->done) for that. Initialize the wait event only once and
reinitialize before adding memory. Unconditionally call complete() and
wait_for_completion_timeout().
If there are no waiters, complete() will only increment ->done - which
will be reset by reinit_completion(). If complete() has already been
called, wait_for_completion_timeout() will not wait.
There is still the chance for a small race between concurrent
reinit_completion() and complete(). If complete() wins, we would not
wait - which is tolerable (and the race exists in current code as well).
Note: We only wait for "some" memory to get onlined, which seems to be
good enough for now.
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Wei Liu <wei.liu@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: linux-hyperv@vger.kernel.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/hv/hv_balloon.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index a02ce43d778d..32e3bc0aa665 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -533,7 +533,6 @@ struct hv_dynmem_device {
* State to synchronize hot-add.
*/
struct completion ol_waitevent;
- bool ha_waiting;
/*
* This thread handles hot-add
* requests from the host as well as notifying
@@ -634,10 +633,7 @@ static int hv_memory_notifier(struct notifier_block *nb, unsigned long val,
switch (val) {
case MEM_ONLINE:
case MEM_CANCEL_ONLINE:
- if (dm_device.ha_waiting) {
- dm_device.ha_waiting = false;
- complete(&dm_device.ol_waitevent);
- }
+ complete(&dm_device.ol_waitevent);
break;
case MEM_OFFLINE:
@@ -726,8 +722,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
has->covered_end_pfn += processed_pfn;
spin_unlock_irqrestore(&dm_device.ha_lock, flags);
- init_completion(&dm_device.ol_waitevent);
- dm_device.ha_waiting = !memhp_auto_online;
+ reinit_completion(&dm_device.ol_waitevent);
nid = memory_add_physaddr_to_nid(PFN_PHYS(start_pfn));
ret = add_memory(nid, PFN_PHYS((start_pfn)),
@@ -753,15 +748,14 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
}
/*
- * Wait for the memory block to be onlined when memory onlining
- * is done outside of kernel (memhp_auto_online). Since the hot
- * add has succeeded, it is ok to proceed even if the pages in
- * the hot added region have not been "onlined" within the
- * allowed time.
+ * Wait for memory to get onlined. If the kernel onlined the
+ * memory when adding it, this will return directly. Otherwise,
+ * it will wait for user space to online the memory. This helps
+ * to avoid adding memory faster than it is getting onlined. As
+ * adding succeeded, it is ok to proceed even if the memory was
+ * not onlined in time.
*/
- if (dm_device.ha_waiting)
- wait_for_completion_timeout(&dm_device.ol_waitevent,
- 5*HZ);
+ wait_for_completion_timeout(&dm_device.ol_waitevent, 5 * HZ);
post_status(&dm_device);
}
}
@@ -1706,6 +1700,7 @@ static int balloon_probe(struct hv_device *dev,
#ifdef CONFIG_MEMORY_HOTPLUG
set_online_page_callback(&hv_online_page);
+ init_completion(&dm_device.ol_waitevent);
register_memory_notifier(&hv_memory_nb);
#endif
--
2.24.1
^ permalink raw reply related
* [PATCH v3 6/8] mm/memory_hotplug: unexport memhp_auto_online
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-hyperv, Michal Hocko, Baoquan He, David Hildenbrand,
Rafael J. Wysocki, Wei Yang, linux-mm, Andrew Morton,
Michal Hocko, linuxppc-dev, Oscar Salvador
In-Reply-To: <20200319131221.14044-1-david@redhat.com>
All in-tree users except the mm-core are gone. Let's drop the export.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
mm/memory_hotplug.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index da6aab272c9b..e21a7d53ade5 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -71,7 +71,6 @@ bool memhp_auto_online;
#else
bool memhp_auto_online = true;
#endif
-EXPORT_SYMBOL_GPL(memhp_auto_online);
static int __init setup_memhp_default_state(char *str)
{
--
2.24.1
^ permalink raw reply related
* [PATCH v3 4/8] powernv/memtrace: always online added memory blocks
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-hyperv, Michal Hocko, Baoquan He, David Hildenbrand,
Rafael J. Wysocki, Greg Kroah-Hartman, Wei Yang, linux-mm,
Paul Mackerras, Andrew Morton, Michal Hocko, linuxppc-dev,
Oscar Salvador
In-Reply-To: <20200319131221.14044-1-david@redhat.com>
Let's always try to online the re-added memory blocks. In case add_memory()
already onlined the added memory blocks, the first device_online() call
will fail and stop processing the remaining memory blocks.
This avoids manually having to check memhp_auto_online.
Note: PPC always onlines all hotplugged memory directly from the kernel
as well - something that is handled by user space on other
architectures.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/powerpc/platforms/powernv/memtrace.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
index d6d64f8718e6..13b369d2cc45 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -231,16 +231,10 @@ static int memtrace_online(void)
continue;
}
- /*
- * If kernel isn't compiled with the auto online option
- * we need to online the memory ourselves.
- */
- if (!memhp_auto_online) {
- lock_device_hotplug();
- walk_memory_blocks(ent->start, ent->size, NULL,
- online_mem_block);
- unlock_device_hotplug();
- }
+ lock_device_hotplug();
+ walk_memory_blocks(ent->start, ent->size, NULL,
+ online_mem_block);
+ unlock_device_hotplug();
/*
* Memory was added successfully so clean up references to it
--
2.24.1
^ permalink raw reply related
* [PATCH v3 3/8] drivers/base/memory: store mapping between MMOP_* and string in an array
From: David Hildenbrand @ 2020-03-19 13:12 UTC (permalink / raw)
To: linux-kernel
Cc: linux-hyperv, Michal Hocko, Baoquan He, David Hildenbrand,
Greg Kroah-Hartman, Rafael J. Wysocki, Wei Yang, linux-mm,
Andrew Morton, Michal Hocko, linuxppc-dev, Oscar Salvador
In-Reply-To: <20200319131221.14044-1-david@redhat.com>
Let's use a simple array which we can reuse soon. While at it, move the
string->mmop conversion out of the device hotplug lock.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/base/memory.c | 38 +++++++++++++++++++++++---------------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index e7e77cafef80..8a7f29c0bf97 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -28,6 +28,24 @@
#define MEMORY_CLASS_NAME "memory"
+static const char *const online_type_to_str[] = {
+ [MMOP_OFFLINE] = "offline",
+ [MMOP_ONLINE] = "online",
+ [MMOP_ONLINE_KERNEL] = "online_kernel",
+ [MMOP_ONLINE_MOVABLE] = "online_movable",
+};
+
+static int memhp_online_type_from_str(const char *str)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(online_type_to_str); i++) {
+ if (sysfs_streq(str, online_type_to_str[i]))
+ return i;
+ }
+ return -EINVAL;
+}
+
#define to_memory_block(dev) container_of(dev, struct memory_block, dev)
static int sections_per_block;
@@ -236,26 +254,17 @@ static int memory_subsys_offline(struct device *dev)
static ssize_t state_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
+ const int online_type = memhp_online_type_from_str(buf);
struct memory_block *mem = to_memory_block(dev);
- int ret, online_type;
+ int ret;
+
+ if (online_type < 0)
+ return -EINVAL;
ret = lock_device_hotplug_sysfs();
if (ret)
return ret;
- if (sysfs_streq(buf, "online_kernel"))
- online_type = MMOP_ONLINE_KERNEL;
- else if (sysfs_streq(buf, "online_movable"))
- online_type = MMOP_ONLINE_MOVABLE;
- else if (sysfs_streq(buf, "online"))
- online_type = MMOP_ONLINE;
- else if (sysfs_streq(buf, "offline"))
- online_type = MMOP_OFFLINE;
- else {
- ret = -EINVAL;
- goto err;
- }
-
switch (online_type) {
case MMOP_ONLINE_KERNEL:
case MMOP_ONLINE_MOVABLE:
@@ -271,7 +280,6 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
ret = -EINVAL; /* should never happen */
}
-err:
unlock_device_hotplug();
if (ret < 0)
--
2.24.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox