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] [day] [month] [year] [list]
Message-ID: <437e07b4-e973-4600-897c-96f67f8ec8a4@lucifer.local>
Date: Tue, 5 Aug 2025 10:30:43 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Adrian Huang <adrianhuang0701@...il.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
        David Hildenbrand <david@...hat.com>, Liam.Howlett@...cle.com,
        Vlastimil Babka <vbabka@...e.cz>, Mike Rapoport <rppt@...nel.org>,
        Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
        Feng Tang <feng.79.tang@...il.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Adrian Huang <ahuang12@...ovo.com>
Subject: Re: [PATCH 1/1] mm: Correct misleading comment on mmap_lock field in
 mm_struct

On Tue, Aug 05, 2025 at 04:47:26PM +0800, Adrian Huang wrote:
> From: Adrian Huang <ahuang12@...ovo.com>
>
> The comment previously described the offset of mmap_lock as 0x120 (hex),
> which is misleading. The correct value is 120 (decimal), and using '0x120'
> could confuse readers trying to understand why the count and owner fields
> reside in separate cachelines.

This isn't the case on my system, pahole puts it at 184, if you subtract the
first field + cache line padding at the start of the structure, then 120 from
there... I guess?

I think it'd be better to say 'offset 56 bytes past the start of the cache
line' rather than to give specific values.

E.g.:

/*
 * Typically the current mmap_lock's offset is 56 bytes from the last cache
   line boundary, which is very optimal... etc. etc.
 ...
 */

>
> This change also removes an unnecessary space for improved formatting.
>
> Fixes: 2e3025434a6b ("mm: relocate 'write_protect_seq' in struct mm_struct")

There's no need for fixes for a comment change.

> Signed-off-by: Adrian Huang <ahuang12@...ovo.com>

Thanks! You're sending from a gmail account though, please make the email you
send from and signed off the same, or otherwise we don't know whether this is
the real you :)

> ---
>  include/linux/mm_types.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 1ec273b06691..ec90bbf22e2b 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -1027,9 +1027,9 @@ struct mm_struct {
>  					     */
>  		/*
>  		 * With some kernel config, the current mmap_lock's offset
> -		 * inside 'mm_struct' is at 0x120, which is very optimal, as
> +		 * inside 'mm_struct' is at 120, which is very optimal, as
>  		 * its two hot fields 'count' and 'owner' sit in 2 different
> -		 * cachelines,  and when mmap_lock is highly contended, both
> +		 * cachelines, and when mmap_lock is highly contended, both
>  		 * of the 2 fields will be accessed frequently, current layout
>  		 * will help to reduce cache bouncing.
>  		 *
> --
> 2.34.1
>

Cheers, Lorenzo

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ