linux-kernel - Re: [PATCH v3] mm/filemap: Allow arch to request folio size for exec memory

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z-WAbWfZzG1GA-4n@casper.infradead.org>
Date: Thu, 27 Mar 2025 16:44:29 +0000
From: Matthew Wilcox <willy@...radead.org>
To: Ryan Roberts <ryan.roberts@....com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	David Hildenbrand <david@...hat.com>,
	Dave Chinner <david@...morbit.com>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	linux-mm@...ck.org
Subject: Re: [PATCH v3] mm/filemap: Allow arch to request folio size for exec
 memory

On Thu, Mar 27, 2025 at 04:06:58PM +0000, Ryan Roberts wrote:
> So let's special-case the read(ahead) logic for executable mappings. The
> trade-off is performance improvement (due to more efficient storage of
> the translations in iTLB) vs potential read amplification (due to
> reading too much data around the fault which won't be used), and the
> latter is independent of base page size. I've chosen 64K folio size for
> arm64 which benefits both the 4K and 16K base page size configs and
> shouldn't lead to any read amplification in practice since the old
> read-around path was (usually) reading blocks of 128K. I don't
> anticipate any write amplification because text is always RO.

Is there not also the potential for wasted memory due to ELF alignment?
Kalesh talked about it in the MM BOF at the same time that Ted and I
were discussing it in the FS BOF.  Some coordination required (like
maybe Kalesh could have mentioned it to me rathere than assuming I'd be
there?)

> +#define arch_exec_folio_order() ilog2(SZ_64K >> PAGE_SHIFT)

I don't think the "arch" really adds much value here.

#define exec_folio_order()	get_order(SZ_64K)

> +#ifndef arch_exec_folio_order
> +/*
> + * Returns preferred minimum folio order for executable file-backed memory. Must
> + * be in range [0, PMD_ORDER]. Negative value implies that the HW has no
> + * preference and mm will not special-case executable memory in the pagecache.
> + */
> +static inline int arch_exec_folio_order(void)
> +{
> +	return -1;
> +}

This feels a bit fragile.  I often expect to be able to store an order
in an unsigned int.  Why not return 0 instead?