linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] mm/mm_init: Fix hash table order logging in alloc_large_system_hash()
@ 2025-10-28 19:10 Isaac J. Manjarres
  2025-10-29 10:03 ` David Hildenbrand
  2025-10-29 11:45 ` Mike Rapoport
  0 siblings, 2 replies; 6+ messages in thread
From: Isaac J. Manjarres @ 2025-10-28 19:10 UTC (permalink / raw)
  To: Mike Rapoport, Andrew Morton
  Cc: Isaac J. Manjarres, stable, kernel-team, linux-mm, linux-kernel

When emitting the order of the allocation for a hash table,
alloc_large_system_hash() unconditionally subtracts PAGE_SHIFT from
log base 2 of the allocation size. This is not correct if the
allocation size is smaller than a page, and yields a negative value
for the order as seen below:

TCP established hash table entries: 32 (order: -4, 256 bytes, linear)
TCP bind hash table entries: 32 (order: -2, 1024 bytes, linear)

Use get_order() to compute the order when emitting the hash table
information to correctly handle cases where the allocation size is
smaller than a page:

TCP established hash table entries: 32 (order: 0, 256 bytes, linear)
TCP bind hash table entries: 32 (order: 0, 1024 bytes, linear)

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org # v5.4+
Signed-off-by: Isaac J. Manjarres <isaacmanjarres@google.com>
---
 mm/mm_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/mm_init.c b/mm/mm_init.c
index 3db2dea7db4c..7712d887b696 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -2469,7 +2469,7 @@ void *__init alloc_large_system_hash(const char *tablename,
 		panic("Failed to allocate %s hash table\n", tablename);
 
 	pr_info("%s hash table entries: %ld (order: %d, %lu bytes, %s)\n",
-		tablename, 1UL << log2qty, ilog2(size) - PAGE_SHIFT, size,
+		tablename, 1UL << log2qty, get_order(size), size,
 		virt ? (huge ? "vmalloc hugepage" : "vmalloc") : "linear");
 
 	if (_hash_shift)
-- 
2.51.1.851.g4ebd6896fd-goog



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] mm/mm_init: Fix hash table order logging in alloc_large_system_hash()
  2025-10-28 19:10 [PATCH v1] mm/mm_init: Fix hash table order logging in alloc_large_system_hash() Isaac J. Manjarres
@ 2025-10-29 10:03 ` David Hildenbrand
  2025-10-29 15:50   ` Isaac Manjarres
  2025-10-29 11:45 ` Mike Rapoport
  1 sibling, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2025-10-29 10:03 UTC (permalink / raw)
  To: Isaac J. Manjarres, Mike Rapoport, Andrew Morton
  Cc: stable, kernel-team, linux-mm, linux-kernel

On 28.10.25 20:10, Isaac J. Manjarres wrote:
> When emitting the order of the allocation for a hash table,
> alloc_large_system_hash() unconditionally subtracts PAGE_SHIFT from
> log base 2 of the allocation size. This is not correct if the
> allocation size is smaller than a page, and yields a negative value
> for the order as seen below:
> 
> TCP established hash table entries: 32 (order: -4, 256 bytes, linear)
> TCP bind hash table entries: 32 (order: -2, 1024 bytes, linear)
> 
> Use get_order() to compute the order when emitting the hash table
> information to correctly handle cases where the allocation size is
> smaller than a page:
> 
> TCP established hash table entries: 32 (order: 0, 256 bytes, linear)
> TCP bind hash table entries: 32 (order: 0, 1024 bytes, linear)
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org # v5.4+

This is a pr_info(), why do you think this is stable material? Just 
curious, intuitively I'd have said that it's not that critical.

> Signed-off-by: Isaac J. Manjarres <isaacmanjarres@google.com>
> ---
>   mm/mm_init.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index 3db2dea7db4c..7712d887b696 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -2469,7 +2469,7 @@ void *__init alloc_large_system_hash(const char *tablename,
>   		panic("Failed to allocate %s hash table\n", tablename);
>   
>   	pr_info("%s hash table entries: %ld (order: %d, %lu bytes, %s)\n",
> -		tablename, 1UL << log2qty, ilog2(size) - PAGE_SHIFT, size,
> +		tablename, 1UL << log2qty, get_order(size), size,

So in case it's smaller than a page we now correctly return "0".

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] mm/mm_init: Fix hash table order logging in alloc_large_system_hash()
  2025-10-28 19:10 [PATCH v1] mm/mm_init: Fix hash table order logging in alloc_large_system_hash() Isaac J. Manjarres
  2025-10-29 10:03 ` David Hildenbrand
@ 2025-10-29 11:45 ` Mike Rapoport
  1 sibling, 0 replies; 6+ messages in thread
From: Mike Rapoport @ 2025-10-29 11:45 UTC (permalink / raw)
  To: Isaac J. Manjarres
  Cc: Andrew Morton, stable, kernel-team, linux-mm, linux-kernel

On Tue, Oct 28, 2025 at 12:10:12PM -0700, Isaac J. Manjarres wrote:
> When emitting the order of the allocation for a hash table,
> alloc_large_system_hash() unconditionally subtracts PAGE_SHIFT from
> log base 2 of the allocation size. This is not correct if the
> allocation size is smaller than a page, and yields a negative value
> for the order as seen below:
> 
> TCP established hash table entries: 32 (order: -4, 256 bytes, linear)
> TCP bind hash table entries: 32 (order: -2, 1024 bytes, linear)
> 
> Use get_order() to compute the order when emitting the hash table
> information to correctly handle cases where the allocation size is
> smaller than a page:
> 
> TCP established hash table entries: 32 (order: 0, 256 bytes, linear)
> TCP bind hash table entries: 32 (order: 0, 1024 bytes, linear)
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org # v5.4+
> Signed-off-by: Isaac J. Manjarres <isaacmanjarres@google.com>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  mm/mm_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index 3db2dea7db4c..7712d887b696 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -2469,7 +2469,7 @@ void *__init alloc_large_system_hash(const char *tablename,
>  		panic("Failed to allocate %s hash table\n", tablename);
>  
>  	pr_info("%s hash table entries: %ld (order: %d, %lu bytes, %s)\n",
> -		tablename, 1UL << log2qty, ilog2(size) - PAGE_SHIFT, size,
> +		tablename, 1UL << log2qty, get_order(size), size,
>  		virt ? (huge ? "vmalloc hugepage" : "vmalloc") : "linear");
>  
>  	if (_hash_shift)
> -- 
> 2.51.1.851.g4ebd6896fd-goog
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] mm/mm_init: Fix hash table order logging in alloc_large_system_hash()
  2025-10-29 10:03 ` David Hildenbrand
@ 2025-10-29 15:50   ` Isaac Manjarres
  2025-10-29 15:58     ` David Hildenbrand
  0 siblings, 1 reply; 6+ messages in thread
From: Isaac Manjarres @ 2025-10-29 15:50 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Mike Rapoport, Andrew Morton, stable, kernel-team, linux-mm,
	linux-kernel

On Wed, Oct 29, 2025 at 11:03:18AM +0100, David Hildenbrand wrote:
> On 28.10.25 20:10, Isaac J. Manjarres wrote:
> > When emitting the order of the allocation for a hash table,
> > alloc_large_system_hash() unconditionally subtracts PAGE_SHIFT from
> > log base 2 of the allocation size. This is not correct if the
> > allocation size is smaller than a page, and yields a negative value
> > for the order as seen below:
> > 
> > TCP established hash table entries: 32 (order: -4, 256 bytes, linear)
> > TCP bind hash table entries: 32 (order: -2, 1024 bytes, linear)
> > 
> > Use get_order() to compute the order when emitting the hash table
> > information to correctly handle cases where the allocation size is
> > smaller than a page:
> > 
> > TCP established hash table entries: 32 (order: 0, 256 bytes, linear)
> > TCP bind hash table entries: 32 (order: 0, 1024 bytes, linear)
> > 
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Cc: stable@vger.kernel.org # v5.4+
> 
> This is a pr_info(), why do you think this is stable material? Just curious,
> intuitively I'd have said that it's not that critical.
> 

Hi David,

Thank you for taking the time to review this patch! I was just under the
impression that any bug--even those for informational logging--should be
sent to stable as well.

> > Signed-off-by: Isaac J. Manjarres <isaacmanjarres@google.com>
> > ---
> >   mm/mm_init.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/mm/mm_init.c b/mm/mm_init.c
> > index 3db2dea7db4c..7712d887b696 100644
> > --- a/mm/mm_init.c
> > +++ b/mm/mm_init.c
> > @@ -2469,7 +2469,7 @@ void *__init alloc_large_system_hash(const char *tablename,
> >   		panic("Failed to allocate %s hash table\n", tablename);
> >   	pr_info("%s hash table entries: %ld (order: %d, %lu bytes, %s)\n",
> > -		tablename, 1UL << log2qty, ilog2(size) - PAGE_SHIFT, size,
> > +		tablename, 1UL << log2qty, get_order(size), size,
> 
> So in case it's smaller than a page we now correctly return "0".

Correct.

> Reviewed-by: David Hildenbrand <david@redhat.com>
> 
> -- 
> Cheers
> 
> David / dhildenb

Thanks!
Isaac


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] mm/mm_init: Fix hash table order logging in alloc_large_system_hash()
  2025-10-29 15:50   ` Isaac Manjarres
@ 2025-10-29 15:58     ` David Hildenbrand
  2025-10-29 16:05       ` Isaac Manjarres
  0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2025-10-29 15:58 UTC (permalink / raw)
  To: Isaac Manjarres
  Cc: Mike Rapoport, Andrew Morton, stable, kernel-team, linux-mm,
	linux-kernel

On 29.10.25 16:50, Isaac Manjarres wrote:
> On Wed, Oct 29, 2025 at 11:03:18AM +0100, David Hildenbrand wrote:
>> On 28.10.25 20:10, Isaac J. Manjarres wrote:
>>> When emitting the order of the allocation for a hash table,
>>> alloc_large_system_hash() unconditionally subtracts PAGE_SHIFT from
>>> log base 2 of the allocation size. This is not correct if the
>>> allocation size is smaller than a page, and yields a negative value
>>> for the order as seen below:
>>>
>>> TCP established hash table entries: 32 (order: -4, 256 bytes, linear)
>>> TCP bind hash table entries: 32 (order: -2, 1024 bytes, linear)
>>>
>>> Use get_order() to compute the order when emitting the hash table
>>> information to correctly handle cases where the allocation size is
>>> smaller than a page:
>>>
>>> TCP established hash table entries: 32 (order: 0, 256 bytes, linear)
>>> TCP bind hash table entries: 32 (order: 0, 1024 bytes, linear)
>>>
>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>> Cc: stable@vger.kernel.org # v5.4+
>>
>> This is a pr_info(), why do you think this is stable material? Just curious,
>> intuitively I'd have said that it's not that critical.
>>
> 
> Hi David,
> 
> Thank you for taking the time to review this patch! I was just under the
> impression that any bug--even those for informational logging--should be
> sent to stable as well.

See https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

In particular:

"
It fixes a problem like an oops, a hang, data corruption, a real 
security issue, a hardware quirk, a build error (but not for things 
marked CONFIG_BROKEN), or some “oh, that’s not good” issue.

Serious issues as reported by a user of a distribution kernel may also 
be considered if they fix a notable performance or interactivity issue. ...
"

-- 
Cheers

David / dhildenb



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] mm/mm_init: Fix hash table order logging in alloc_large_system_hash()
  2025-10-29 15:58     ` David Hildenbrand
@ 2025-10-29 16:05       ` Isaac Manjarres
  0 siblings, 0 replies; 6+ messages in thread
From: Isaac Manjarres @ 2025-10-29 16:05 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Mike Rapoport, Andrew Morton, stable, kernel-team, linux-mm,
	linux-kernel

On Wed, Oct 29, 2025 at 04:58:37PM +0100, David Hildenbrand wrote:
> On 29.10.25 16:50, Isaac Manjarres wrote:
> > On Wed, Oct 29, 2025 at 11:03:18AM +0100, David Hildenbrand wrote:
> > > On 28.10.25 20:10, Isaac J. Manjarres wrote:
> > > > When emitting the order of the allocation for a hash table,
> > > > alloc_large_system_hash() unconditionally subtracts PAGE_SHIFT from
> > > > log base 2 of the allocation size. This is not correct if the
> > > > allocation size is smaller than a page, and yields a negative value
> > > > for the order as seen below:
> > > > 
> > > > TCP established hash table entries: 32 (order: -4, 256 bytes, linear)
> > > > TCP bind hash table entries: 32 (order: -2, 1024 bytes, linear)
> > > > 
> > > > Use get_order() to compute the order when emitting the hash table
> > > > information to correctly handle cases where the allocation size is
> > > > smaller than a page:
> > > > 
> > > > TCP established hash table entries: 32 (order: 0, 256 bytes, linear)
> > > > TCP bind hash table entries: 32 (order: 0, 1024 bytes, linear)
> > > > 
> > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > > Cc: stable@vger.kernel.org # v5.4+
> > > 
> > > This is a pr_info(), why do you think this is stable material? Just curious,
> > > intuitively I'd have said that it's not that critical.
> > > 
> > 
> > Hi David,
> > 
> > Thank you for taking the time to review this patch! I was just under the
> > impression that any bug--even those for informational logging--should be
> > sent to stable as well.
> 
> See https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> 
> In particular:
> 
> "
> It fixes a problem like an oops, a hang, data corruption, a real security
> issue, a hardware quirk, a build error (but not for things marked
> CONFIG_BROKEN), or some “oh, that’s not good” issue.
> 
> Serious issues as reported by a user of a distribution kernel may also be
> considered if they fix a notable performance or interactivity issue. ...
> "
> 
> -- 
> Cheers
> 
> David / dhildenb
> 

Thank you for pointing that out, sorry about that. I'll keep that in
mind moving forward.

Thanks,
Isaac


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-10-29 16:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-28 19:10 [PATCH v1] mm/mm_init: Fix hash table order logging in alloc_large_system_hash() Isaac J. Manjarres
2025-10-29 10:03 ` David Hildenbrand
2025-10-29 15:50   ` Isaac Manjarres
2025-10-29 15:58     ` David Hildenbrand
2025-10-29 16:05       ` Isaac Manjarres
2025-10-29 11:45 ` Mike Rapoport

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox