Re: [PATCH v6 0/4] mm: introduce THP deferred setting

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

I think the TL;DR here to avoid too much back and forth is - let's please
make this super super simple :)

I would prefer anything that has a dependency to just sit in RFC until the
dependency is merged.

Or, alternatively, to have a big note at the top:

ANDREW - Please do not merge in mm-unstable until series [1] is merged, and
when that is merged please ping for a resend.

Or whatever it might be.

On Wed, May 21, 2025 at 01:46:38PM +0200, David Hildenbrand wrote:
> On 21.05.25 13:24, Lorenzo Stoakes wrote:
> > To start with I do apologise for coming to this at v6, I realise it's
> > irritating to have push back at this late stage. This is more so my attempt
> > to understand where this series -sits- so I can properly review it.
> >
> > So please bear with me here :)
> >
> > So, I remain very confused. This may just be a _me_ thing here :)
> >
> > So let me check my understanding:
> >
> > 1. This series introduces this new THP deferred mode.
> > 2. By 'follow-up' really you mean 'inspired by' or 'related to' right?
> > 3. If this series lands before [1], commits 2 - 4 are 'undefined
> >     behaviour'.
> >
> > In my view if 3 is true this series should be RFC until [1] merges.
> >
> > If I've got it wrong and this needs to land first, we should RFC [1].
> >
> > That way we can un-RFC once the dependency is met.
>
> I really don't have a strong opinion on the RFC vs. !RFC like others here --
> as long as the dependency is obvious. I treat RFC more as a "rough idea"
> than well tested work.
>
> Anyhow, to me the dependency is obvious, but I've followed the MM meeting
> discussions, development etc.

Right but is it clear to Andrew? I mean the cover letter was super unclear
to me.

What's to prevent things getting merged out of order? And do people 'just
have to remember' to resend? And a resend doesn't necessarily mean patch
set X will come after patch set Y.

If there's a requirement related to the ordering of these series it really
has to be expressed very clearly.

(by the way, I feel expressing things like this is a kind of area where we
have _some kind_ of a break down in kernel process or it'd be nice to have
tags or something to properly express this sort of thing. But maybe another
discussion :)

>
> I interpret "follow up" as "depends on" here. Likely we should have spelled
> out "This series depends on the patch series X that was not merged yet, and
> likely a new version will be required once merged.".
>
> In this particular case, maybe we should just have sent one initial RFC, and
> then rebased it on top of the other work on a public git branch (linked from
> the RFC cover letter).
>
> Once the dependency gets merged, we could just resend the series. Looking at
> the changelog, only minor stuff changed (mostly rebasing etc).
>
> Moving forward, I don't think there is the need to resend as long as the
> dependency isn't merged upstream (or close to being merged upstream) yet.

I mean this is still 'just have to remember' stuff :)

Do we need patches 2-4 if the dependency isn't merged? That was unclear to
me.

>
> > > > Because again... that surely makes this series a no-go until we land the
> > > > prior (which might be changed, and thus necessitate re-testing).
> > > >
> > > > Are you going to provide any of these numbers/data anywhere?
> > > There is a link to the results in this cover letter
> > > [3] - https://people.redhat.com/npache/mthp_khugepaged_defer/testoutput2/output.html
> > > >
> >
> > Ultimately it's not ok in mm to have a link to a website that might go away
> > any time, these cover letters are 'baked in' to the commit log. Are you
> > sure this website with 'testoutput2' will exist in 10 years time? :)
> >
> > You should at the very least add a summary of this data in the cover
> > letter, perhaps referring back to this link as 'at the time of writing full
> > results are available at' or something like this.
>
> Yeah, or of they were included in some other mail, we can link to that mail
> in lore.
>
> --
> Cheers,
>
> David / dhildenb
>