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

[David Hildenbrand <david@redhat.com>]

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.

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.

>>> 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