[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <65dc19c3-b2de-4e56-8f11-b8185f721c4e@lucifer.local>
Date: Mon, 30 Jun 2025 12:53:10 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Ryan Roberts <ryan.roberts@....com>
Cc: Dev Jain <dev.jain@....com>, akpm@...ux-foundation.org, david@...hat.com,
willy@...radead.org, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
catalin.marinas@....com, will@...nel.org, Liam.Howlett@...cle.com,
vbabka@...e.cz, jannh@...gle.com, anshuman.khandual@....com,
peterx@...hat.com, joey.gouly@....com, ioworker0@...il.com,
baohua@...nel.org, kevin.brodsky@....com, quic_zhenhuah@...cinc.com,
christophe.leroy@...roup.eu, yangyicong@...ilicon.com,
linux-arm-kernel@...ts.infradead.org, hughd@...gle.com,
yang@...amperecomputing.com, ziy@...dia.com
Subject: Re: [PATCH v4 1/4] mm: Optimize mprotect() for MM_CP_PROT_NUMA by
batch-skipping PTEs
On Mon, Jun 30, 2025 at 12:39:33PM +0100, Ryan Roberts wrote:
> On 30/06/2025 12:25, Lorenzo Stoakes wrote:
> > On Sat, Jun 28, 2025 at 05:04:32PM +0530, Dev Jain wrote:
> >> In case of prot_numa, there are various cases in which we can skip to the
> >> next iteration. Since the skip condition is based on the folio and not
> >> the PTEs, we can skip a PTE batch. Additionally refactor all of this
> >> into a new function to clean up the existing code.
> >
> > Hmm, is this a completely new concept for this series?
> >
> > Please try not to introduce brand new things to a series midway through.
> >
> > This seems to be adding a whole ton of questionable logic for an edge case.
> >
> > Can we maybe just drop this for this series please?
>
> From my perspective, at least, there are no new logical changes in here vs the
> previous version. And I don't think the patches have been re-organised either.
> David (I think?) was asking for the name of the patch to be changed to include
> MM_CP_PROT_NUMA and also for the code to be moved out of line to it's own
> function. That's all that Dev has done AFAICT (although as per my review
> comments, the refactoring has introduced a bug).
>
> My preference is that we should ultimately support this batching. It could be a
> separate series if you insist, but it's all contbuting to the same goal
> ultimately; making mprotect support PTE batching.
>
> Just my 2c.
>
> Thanks,
> Ryan
Ack, was my mistake, apologies. I hadn't realised this was part of the series, I
hadn't looked it for a while...
But I think it's better to have the refactor and the batch bit done separately
so it's clear which is which, unless the change is so trivial as for that to be
just noise.
Cheers, Lorenzo
Powered by blists - more mailing lists