* [PATCH] mm/percpu.c: Modify calculation of size of populated bitmap of chunk for memory allocation
@ 2020-09-03 12:41 mcsmonk
2020-09-03 13:46 ` Vlastimil Babka
0 siblings, 1 reply; 3+ messages in thread
From: mcsmonk @ 2020-09-03 12:41 UTC (permalink / raw)
To: linux-mm; +Cc: Sunghyun Jin
From: Sunghyun Jin <mcsmonk@gmail.com>
Variable populated, which is a member of struct pcpu_chunk, is used as a
unit of size of unsigned long.
However, size of populated is miscounted. So, I fix this minor part.
Signed-off-by: Sunghyun Jin <mcsmonk@gmail.com>
---
mm/percpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/percpu.c b/mm/percpu.c
index f4709629e6de..1ed1a349eab8 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -1316,7 +1316,7 @@ static struct pcpu_chunk * __init pcpu_alloc_first_chunk(unsigned long tmp_addr,
/* allocate chunk */
alloc_size = sizeof(struct pcpu_chunk) +
- BITS_TO_LONGS(region_size >> PAGE_SHIFT);
+ BITS_TO_LONGS(region_size >> PAGE_SHIFT) * sizeof(unsigned long);
chunk = memblock_alloc(alloc_size, SMP_CACHE_BYTES);
if (!chunk)
panic("%s: Failed to allocate %zu bytes\n", __func__,
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mm/percpu.c: Modify calculation of size of populated bitmap of chunk for memory allocation
2020-09-03 12:41 [PATCH] mm/percpu.c: Modify calculation of size of populated bitmap of chunk for memory allocation mcsmonk
@ 2020-09-03 13:46 ` Vlastimil Babka
2020-09-03 15:46 ` Dennis Zhou
0 siblings, 1 reply; 3+ messages in thread
From: Vlastimil Babka @ 2020-09-03 13:46 UTC (permalink / raw)
To: mcsmonk, linux-mm; +Cc: Mike Rapoport, Dennis Zhou (Facebook), Tejun Heo
On 9/3/20 2:41 PM, mcsmonk wrote:
> From: Sunghyun Jin <mcsmonk@gmail.com>
>
> Variable populated, which is a member of struct pcpu_chunk, is used as a
> unit of size of unsigned long.
> However, size of populated is miscounted. So, I fix this minor part.
+CC folks who touched it last
Nice find! Did you observe e.g. a panic that can be used in the commit log? Or
were we always lucky thanks to alignment?
Is there perhaps a commit that introduced the bug and we can use it as Fixes:?
My brief look suggests 8ab16c43ea79 ("percpu: change the number of pages marked
in the first_chunk pop bitmap")
Thanks!
> Signed-off-by: Sunghyun Jin <mcsmonk@gmail.com>
> ---
> mm/percpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/percpu.c b/mm/percpu.c
> index f4709629e6de..1ed1a349eab8 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -1316,7 +1316,7 @@ static struct pcpu_chunk * __init pcpu_alloc_first_chunk(unsigned long tmp_addr,
>
> /* allocate chunk */
> alloc_size = sizeof(struct pcpu_chunk) +
> - BITS_TO_LONGS(region_size >> PAGE_SHIFT);
> + BITS_TO_LONGS(region_size >> PAGE_SHIFT) * sizeof(unsigned long);
> chunk = memblock_alloc(alloc_size, SMP_CACHE_BYTES);
> if (!chunk)
> panic("%s: Failed to allocate %zu bytes\n", __func__,
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mm/percpu.c: Modify calculation of size of populated bitmap of chunk for memory allocation
2020-09-03 13:46 ` Vlastimil Babka
@ 2020-09-03 15:46 ` Dennis Zhou
0 siblings, 0 replies; 3+ messages in thread
From: Dennis Zhou @ 2020-09-03 15:46 UTC (permalink / raw)
To: Vlastimil Babka; +Cc: mcsmonk, linux-mm, Mike Rapoport, Dennis Zhou, Tejun Heo
Hello Vlastimil and Sunghyun,
On Thu, Sep 03, 2020 at 03:46:33PM +0200, Vlastimil Babka wrote:
> On 9/3/20 2:41 PM, mcsmonk wrote:
> > From: Sunghyun Jin <mcsmonk@gmail.com>
> >
> > Variable populated, which is a member of struct pcpu_chunk, is used as a
> > unit of size of unsigned long.
> > However, size of populated is miscounted. So, I fix this minor part.
>
> +CC folks who touched it last
>
Thanks for CCing me.
> Nice find! Did you observe e.g. a panic that can be used in the commit log? Or
> were we always lucky thanks to alignment?
Well that is indeed awkward. Luckily the first chunk is a bit special
and only holds 7 dynamic pages. Additionally, the allocation rounds to
SMP_CACHE_BYTES so that would give us 8 bytes to play with as
struct pcpu_chunk is 120 bytes. So, while technically (wrong) the
1 byte was sufficient and the additional buffer is why at least I never
got a panic report to today.
> Is there perhaps a commit that introduced the bug and we can use it as Fixes:?
> My brief look suggests 8ab16c43ea79 ("percpu: change the number of pages marked
> in the first_chunk pop bitmap")
>
That is the right commit. I'll pick this up and add the fixes line.
> Thanks!
>
Thanks,
Dennis
> > Signed-off-by: Sunghyun Jin <mcsmonk@gmail.com>
> > ---
> > mm/percpu.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/percpu.c b/mm/percpu.c
> > index f4709629e6de..1ed1a349eab8 100644
> > --- a/mm/percpu.c
> > +++ b/mm/percpu.c
> > @@ -1316,7 +1316,7 @@ static struct pcpu_chunk * __init pcpu_alloc_first_chunk(unsigned long tmp_addr,
> >
> > /* allocate chunk */
> > alloc_size = sizeof(struct pcpu_chunk) +
> > - BITS_TO_LONGS(region_size >> PAGE_SHIFT);
> > + BITS_TO_LONGS(region_size >> PAGE_SHIFT) * sizeof(unsigned long);
> > chunk = memblock_alloc(alloc_size, SMP_CACHE_BYTES);
> > if (!chunk)
> > panic("%s: Failed to allocate %zu bytes\n", __func__,
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-03 15:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-03 12:41 [PATCH] mm/percpu.c: Modify calculation of size of populated bitmap of chunk for memory allocation mcsmonk
2020-09-03 13:46 ` Vlastimil Babka
2020-09-03 15:46 ` Dennis Zhou
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.