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] [thread-next>] [day] [month] [year] [list]
Message-ID: <fea0d5cd-ad63-4fcb-a425-bd9b0d598006@lucifer.local>
Date: Thu, 17 Jul 2025 10:19:30 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Hugh Dickins <hughd@...gle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
        Baolin Wang <baolin.wang@...ux.alibaba.com>,
        Matthew Wilcox <willy@...radead.org>, david@...hat.com, ziy@...dia.com,
        Liam.Howlett@...cle.com, npache@...hat.com, ryan.roberts@....com,
        dev.jain@....com, baohua@...nel.org, vbabka@...e.cz, rppt@...nel.org,
        surenb@...gle.com, mhocko@...e.com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] mm: fault in complete folios instead of individual
 pages for tmpfs

TL;DR - I am fine with this not being backported.

On Tue, Jul 15, 2025 at 01:03:40PM -0700, Hugh Dickins wrote:
> On Wed, 9 Jul 2025, Lorenzo Stoakes wrote:
> > Well :) you say yourself it was an oversight, and it very clearly has a perf
> > _impact_, which if you compare backwards to acd7ccb284b8 is a degradation, but I
> > get your point.
> >
> > However, since you say 'oversight' this seems to me that you really meant to
> > have included it but hadn't noticed, and additionally, since it just seems to be
> > an unequivical good - let's maybe flip this round - why NOT backport it to
> > stable?
>
> I strongly agree with Baolin: this patch is good, thank you, but it is
> a performance improvement, a new feature, not a candidate for the stable
> tree.  I'm surprised anyone thinks otherwise: Andrew, please delete that
> stable tag before advancing the patch from mm-unstable to mm-stable.

I understand that we don't arbitrarily backport perf improvements,
obviously :)

But at some point here Baolin said (though perhaps I misinterpreted) this
was an oversight that he left out to be done then, in effect.

But I'm fine for us to decide to treat this differently, I'm not going to
keep pushing this - it's all good - let's not backport.

>
> And the Fixee went into 6.14, so it couldn't go to 6.12 LTS anyway.

On my part I was talking about a standard stable backport not somehow
arbitrarily shoving the two patches into 6.12 LTS... to be clear!

>
> An unequivocal good? I'm not so sure.
>
> I expect it ought to be limited, by fault_around_bytes (or suchlike).

Yeah...

>
> If I understand all the mTHP versus large folio versus PMD-huge handling
> correctly (and of course I do not, I'm still weeks if not months away
> from understanding most of it), the old vma_is_anon_shmem() case would
> be limited by the shmem mTHP tunables, and one can reasonably argue that
> they would already take fault_around_bytes-like considerations into account;
> but the newly added file-written cases are governed by huge= mount options
> intended for PMD-size, but (currently) permitting all lesser orders.
> I don't think that mounting a tmpfs huge=always implies that mapping
> 256 PTEs for one fault is necessarily a good strategy.
>
> But looking in the opposite direction, why is there now a vma_is_shmem()
> check there in finish_fault() at all?  If major filesystems are using
> large folios, why aren't they also allowed to benefit from mapping
> multiple PTEs at once (in this shared-writable case which the existing
> fault-around does not cover - I presume to avoid write amplification,
> but that's not an issue when the folio is large already).

This seems like something we should tread carefully around.

>
> It's fine to advance cautiously, keeping this to shmem in coming release;
> but I think it should be extended soon (without any backport to stable),
> and consideration given to limiting it.

Yes agreed!

>
> Hugh

Cheers, Lorenzo

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ