Message-ID: <79525362-2377-441b-8575-d2307bd77f26@arm.com>
Date: Mon, 30 Jun 2025 10:18:47 +0100
From: Ryan Roberts <ryan.roberts@....com>
To: David Hildenbrand <david@...hat.com>, Dev Jain <dev.jain@....com>,
 linux-kernel@...r.kernel.org
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
 "Liam R. Howlett" <Liam.Howlett@...cle.com>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 Vlastimil Babka <vbabka@...e.cz>, Jann Horn <jannh@...gle.com>,
 Mike Rapoport <rppt@...nel.org>, Suren Baghdasaryan <surenb@...gle.com>,
 Michal Hocko <mhocko@...e.com>, Zi Yan <ziy@...dia.com>,
 Matthew Brost <matthew.brost@...el.com>,
 Joshua Hahn <joshua.hahnjy@...il.com>, Rakie Kim <rakie.kim@...com>,
 Byungchul Park <byungchul@...com>, Gregory Price <gourry@...rry.net>,
 Ying Huang <ying.huang@...ux.alibaba.com>,
 Alistair Popple <apopple@...dia.com>, Pedro Falcato <pfalcato@...e.de>,
 Rik van Riel <riel@...riel.com>, Harry Yoo <harry.yoo@...cle.com>
Subject: Re: [PATCH v1 1/4] mm: convert FPB_IGNORE_* into FPB_HONOR_*

On 30/06/2025 10:08, David Hildenbrand wrote:
> On 30.06.25 11:04, Ryan Roberts wrote:
>> On 30/06/2025 04:34, Dev Jain wrote:
>>>
>>> On 29/06/25 2:30 am, David Hildenbrand wrote:
>>>> On 28.06.25 05:37, Dev Jain wrote:
>>>>>
>>>>> On 27/06/25 5:25 pm, David Hildenbrand wrote:
>>>>>> Honoring these PTE bits is the exception, so let's invert the meaning.
>>>>>>
>>>>>> With this change, most callers don't have to pass any flags.
>>>>>>
>>>>>> No functional change intended.
>>>>>
>>>>> FWIW I had proposed this kind of change earlier to Ryan (CCed) and
>>>>> he pointed out: "Doesn't that argument apply in reverse if you want
>>>>> to ignore something new in future?
>>>>>
>>>>> By default we are comparing all the bits in the pte when determining the
>>>>> batch.
>>>>> The flags request to ignore certain bits.
>>>>
>>>> That statement is not true: as default we ignore the write and young bit. And
>>>> we don't have flags for that ;)
>>>>
>>>> Now we also ignore the dirty and soft-dity bit as default, unless told not to
>>>> do that by one very specific caller.
>>>>
>>>>> If we want to ignore extra bits in
>>>>> future, we add new flags and the existing callers don't need to be updated.
>>>>
>>>> What stops you from using FPB_IGNORE_* for something else in the future?
>>>>
>>>> As a side note, there are not that many relevant PTE bits to worry about in
>>>> the near future ;)
>>>>
>>>> I mean, uffd-wp, sure, ... and before we add a FPB_HONOR_UFFD_WP to all users
>>>> to be safe (and changing the default to ignore), you could add a
>>>> FPB_IGNORE_UFFD_WP first, to then check who really can tolerate just ignoring
>>>> it (most of them, I assume).
>>> I agree.
>>
>> Meh. Personally I think if you start mixing HONOR and IGNORE flags, it becomes
>> very confusing to work out what is being checked for and what is not. I stand by
>> my original view. But yeah, writable and young confuse it a bit... How about
>> generalizing by explicitly requiring IGNORE flags for write and young, then also
>> create a flags macro for the common case?
>>
>> #define FPB_IGNORE_COMMON (FPB_IGNORE_WRITE | FPB_IGNORE_YOUNG |    \
>>                FPB_IGNORE_DIRTY | FPB_IGNORE_SOFT_DIRTY)
>>
>> It's not a hill I'm going to die on though...
> 
> How about we make this function simpler, not more complicated? ;)

I think here we both have different views of what is simpler... You are trying
to optimize for the callers writing less code. I'm trying to optimize for the
reader to be able to easily determine what the function will do for a given caller.

But it's your name above the door :) I don't want to bikeshed, so go ahead
change to HONOR.

Powered by blists - more mailing lists