Re: [PATCH v6 23/26] mm: Remove pXX_devmap callers

On Tue, Jan 14, 2025 at 10:50:49AM -0800, Dan Williams wrote:

Alistair Popple wrote:

quoted

The devmap PTE special bit was used to detect mappings of FS DAX
pages. This tracking was required to ensure the generic mm did not
manipulate the page reference counts as FS DAX implemented it's own
reference counting scheme.

Now that FS DAX pages have their references counted the same way as
normal pages this tracking is no longer needed and can be
removed.

Almost all existing uses of pmd_devmap() are paired with a check of
pmd_trans_huge(). As pmd_trans_huge() now returns true for FS DAX pages
dropping the check in these cases doesn't change anything.

However care needs to be taken because pmd_trans_huge() also checks that
a page is not an FS DAX page. This is dealt with either by checking
!vma_is_dax() or relying on the fact that the page pointer was obtained
from a page list. This is possible because zone device pages cannot
appear in any page list due to sharing page->lru with page->pgmap.

While the patch looks straightforward I think part of taking "care" in
this case is to split it such that any of those careful conversions have
their own bisect point in the history.

Perhaps this can move to follow-on series to not blow up the patch count
of the base series? ...but first want to get your reaction to splitting
for bisect purposes.

TBH I don't feel too strongly about it - I suppose it would make it easier to
bisect to the specific case we weren't careful enough about. However I think if
a bug is bisected to this particular patch it would be relatively easy based on
the context of the bug to narrow it down to a particular file or two.

I do however feel strongly about whether or not that should be done in a
follow-on series :-)

Rebasing such a large series has already become painful and error prone enough
so if we want to split this change up it will definitely need to be a separate
series done once the rest of this has been merged. So I could be pursaded to
roll this and the pfn_t removal (as that depends on devmap going away) together.

Let me know what you think.