Re: [PATCH v1 1/4] mm: convert FPB_IGNORE_* into FPB_HONOR_*
From: David Hildenbrand
Date: Mon Jun 30 2025 - 05:08:50 EST
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? ;)
--
Cheers,
David / dhildenb