On 06/10/2023 16:24, Yu Zhao wrote:
> On Fri, Oct 6, 2023 at 2:53 AM Ryan Roberts <ryan.roberts@xxxxxxx> wrote:
>>
>> On 05/10/2023 20:23, Yu Zhao wrote:
>>> On Thu, Oct 5, 2023 at 2:12 AM David Hildenbrand <david@xxxxxxxxxx> 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.
>
> From my POV, there have been some random changes between each version
> of your patchset:
> 1. v3 *introduced* a boot parameter `flexthp_unhinted_max`, which
> *disappeared* in later versions.
> 2. v4 *introduced* a random constant ANON_FOLIO_MAX_ORDER_UNHINTED,
> which *disappeared* in v6.
> 3. v5 is really close to what I think can be merged -- my only
> objection is to ANON_FOLIO_MAX_ORDER_UNHINTED, I've never heard
> *a good reason* why that random constant should sit in core mm.
> 4. v6 *introduced* a sysfs ABI -- we had a couple of alignment meetings
> about how it might look but there was *never a consensus*.
A couple of points; It was my understanding that the preferred way of working
was to turn up with real code, that could be used as context for discussion.
That's the approach I've been taking. If this is not how I should be doing it,
then perhaps you could point me to the preferred approach?
For me, none of these changes are "random"; they are all attempts to solve the
problem of internal fragmentation when the HW-preferred order is "too big" to
have any reasonable confidence that we won't be wasting loads of memory. And
1,2,3 was trying to do this without introducing any ABI, as per your preference.
The reasons for their removal (and moving on to another approach) was due to the
feedback that you linked below.
I appreciate in hindsight, that you have found this meandering frustrating. But
if nothing else, it has been beneficial to help me understand the expected
standards - hopefully I'm learning.
>
> FWICT, you haven't made up your mind. So I'm not convinced this series
> is close to being ready to introduce an ABI. This is NOT a NAK, just a
> ton of concerns with your current approach.
>
> And I'm not sure how I could be more clear on how we can move forward
> progress [1][2][3]. Some suggestions would also help.
I appreciate the feedback you have given so far, and as far as I'm concerned,
I've listened to it; the changes you enumerated above are a direct results of
your feedback in the links below. And I do hope you continue to provide feedback
and help guide me towards a better solution.
But, there are questions I asked in the previous email which you did not answer.
If you are able to answer them, that would help me be better aligned with your
thinking. Namely:
- You said the priorities are wrong and there are many pending work items I
should be looking at first - what were you referring to here?
- If you want a compile time option for testing, what is preventing you doing
testing by merging in the patches? What is the benefit of it being upstream?
I see that both you and David have now responded directly to the series - thanks
to both of you. I will read and respond to that shortly, and let's continue the
conversation!
Thanks,
Ryan
>
> [1] v3 https://lore.kernel.org/linux-mm/CAOUHufaDfJwF_-zb6zV5COG-KaaGcSyrNmbaEzaWz2UjcGGgHQ@xxxxxxxxxxxxxx/
> [2] v4 https://lore.kernel.org/linux-mm/CAOUHufackQzy+yXOzaej+G6DNYK-k9GAUHAK6Vq79BFHr7KwAQ@xxxxxxxxxxxxxx/
> [3] v5 https://lore.kernel.org/linux-mm/CAOUHufbUGwc2XvZOBmTCzMsOHxP-eLB60EdysKYzrkRMScOyMg@xxxxxxxxxxxxxx/
