Re: [PATCH mm-unstable v2 00/16] mm: Introduce arch_mmap_hint()

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

On Fri, Dec 13, 2024 at 10:06:55AM -0500, Kalesh Singh wrote:
> On Fri, Dec 13, 2024 at 4:00 AM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> >
> > On Thu, Dec 12, 2024 at 05:36:09PM -0800, Andrew Morton wrote:
> > > On Thu, 12 Dec 2024 22:51:34 +0000 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
> > >
> > > > You've fundamentally violated kernel process and etiquette. I'd be more
> > > > forgiving, but this is at v2 and you've not cc'd KEY people. Twice. This is
> > > > totally unacceptable. See [0] if you are unsure of how to do so.
> > >
> > > This feels excessive to me.  linux-mm averages a mere 140 mesages/day
> > > and it seems reasonable to assume that key people are spending their 5
> > > minutes to scroll through the email subjects.
> >
> > In practice we did all miss it, and I don't think it's unreasonable to ask
> > people to run get_maintainers.pl to avoid this.
> >
> > In any case, I truly do think this series works better as RFC, I mean Liam
> > has already voiced the kind of disagreements I share with it, and we need
> > to rethink how to approach it in general.
> >
> > So if this is simply sent as RFC with the correct cc's (and ideally with
> > some review feedback applied - a better cover letter, etc.) then it makes
> > everything easier.
> >
> > As mentioned the timing is unfortunate here, this is a series we really
> > want to make sure is properly reviewed before any chance of merge so again
> > this points to RFC being the way forward.
>
> Hi everyone,
>
> Sorry for the delayed response -- I was traveling and didn’t have
> access to email.

Ack.

>
> Thank you for the feedback. I realize I missed some key reviewers in
> the CC list for this patch.
> When I ran get_maintainer.pl, it returned a large list of recipients.
> To avoid over-CC’ing people (which has been an issue for me in the
> past), I tried to trim it down to maintainers and a few others I
> thought would be interested. Clearly, I got it wrong and missed some
> key folks. That was not my intention, and I’ll make sure to fix it
> when I resend the patch as an RFC.

Sure thanks :) Much appreciated. Sorry to be so curt there, just I think
important to underline.

We just want to make sure we can find a way to get this series sorted out
so we can get it merged in a form that makes sense overall, ultimately! :)

>
> On the technical side, Liam is right that the copy-pasted arch code
> has inconsistencies (missing checks, order of checks, ...). I agree
> there’s room for further consolidation. I’ll take another stab at it
> and resend it as an RFC with an updated cover letter, as Lorenzo and
> others suggested.

The most useful thing here as well to help us understand would be to write
more in the cover letter to expand on what it is you ultimately what to
achieve here - it seems like an extension on the existing THP work on a
per-arch basis (I may be wrong)? So adding more detail would be super
useful here! :)

We do hope to avoid arch hooks if at all possible explicitly for the reason
that they can be applied at unfortunate times in terms of locking/whether
the objects in question are fully/partially instantiated, VMA visibility
etc. etc. based on having had issues in these areas before.

Also if a hook means 'anything' can happen at a certain point, it means we
can't make any assumptions about what has/hasn't and have to account for
anything which seriously complicates things.

Ideally we'd find a means to achieve the same thing while also exposing us
as little as possible to what may be mutated.

>
> Thanks,
> Kalesh

Thanks!