Re: + mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch added to mm-unstable branch

[Ryan Roberts <ryan.roberts@arm.com>]

On 05/10/2023 20:23, Yu Zhao wrote:
> On Thu, Oct 5, 2023 at 2:12 AM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 29.09.23 21:08, Andrew Morton wrote:
>>>
>>> The patch titled
>>>       Subject: mm: non-pmd-mappable, large folios for folio_add_new_anon_rmap()
>>> has been added to the -mm mm-unstable branch.  Its filename is
>>>       mm-non-pmd-mappable-large-folios-for-folio_add_new_anon_rmap.patch
>>>
>>
>> Andrew, please don't move these patches to stable before we have a
>> couple of acks from people involved in the discussions.
>>
>> I'm confident that we'll get them into 6.8.
> 
> I am not :)

I don't find that comment particularly helpful; Are you implying a NAK? If so,
some rationale and actionable suggestions would help.

> 
> As I repeated, the priorities seem wrong to me. There are many pending
> work items that don't involve any ABI changes

Are you referring to the prerequisite list [1]? If so, then its my understanding
that these work items are all in progress:

  - David is working on "shared vs exclusive mappings"
  - Zi Yan has posted an RFC for compaction
  - Yin Fengwei's mlock series is now in mm-stable
  - Yin Fengwei's madvise series is in 6.6
  - I've reworked and posted a series for deferred_split_folio; although I've
    deprioritied it because you said it wasn't really a pre-requisite.
  - numa balancing depends on David's "shared vs exclusive mappings" work
  - I've started looking at "large folios in swap cache" in the background,
    because I'm seeing some slow down with large folios, but we also agreed that
    wasn't a prerequisite

Or if that's not what you are referring to, then perhaps you are referring to
your comments against v5, which I thought were addressed in v6:

  - Removal of the hardcoded order policy [2]; which is removed and replaced by
    the flexibility of user-selectable orders (this has proven very useful for
    my testing)

, and IMO, we should
> focus on them before we even start discussing ABI changes.

I don't see why the prerequisites can't be done in parallel. I don't see any
dependency between them and the ABI.


> 
> I've been quiet  because I don't think opinion based discussions are
> essential to nailing down ABIs. 

We had 2 mm alignment meetings focussing on this topic, where the resounding
conclusion that I heard was that at minimum, we need a way to enable/disable
this at runtime. I'll admit that what I've posted goes a bit further than that,
but I was hoping we could argue about that in the context of the patch set. But
I definitely think we have moved on from the "we don't need any user ABI for
now" position.

As I suggested, I prefer a Kconfig
> option as the first step so that people can play with it. After we
> have collected field data, then we'd be better at coming up with
> suitable ABIs.

If you want to make it a compile time option only, and use it for testing, then
why do you need it upstream at all? Why not just take the patches and apply
them? Then we could use your data to help guide the upstreaming (I have asked
this before, but never saw a response [3]). Personally though, I would rather
not let perfection get in the way of good enough - I can see real world wins
with this and would prefer to start getting it into mainline incrementally,
sooner rather than later.

Thanks,
Ryan


[1] https://lore.kernel.org/linux-mm/f8d47176-03a8-99bf-a813-b5942830fd73@arm.com/
[2]
https://lore.kernel.org/linux-mm/CAOUHufbUGwc2XvZOBmTCzMsOHxP-eLB60EdysKYzrkRMScOyMg@mail.gmail.com/
[3] https://lore.kernel.org/linux-mm/e80cd2e6-6f7c-4337-a170-152926863290@arm.com/