| Message ID | 20200903124116.1668-1-mcsmonk@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/percpu.c: Modify calculation of size of populated bitmap of chunk for memory allocation | expand |
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__, >
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__, > > >
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__,