On 2024/7/7 10:11, Andrew Morton wrote:
On Sat, 6 Jul 2024 14:29:00 -0700 (PDT) Hugh Dickins <hughd@xxxxxxxxxx> wrote:
What you show above is exactly what I had when I was originally testing
over the top of mm-everything (well, not quite exactly, I don't think I
bothered with the data_race()). But I grew to feel that probably everyone
else would be happier with less of those internals _deferred_list and
__folio_undo_large_rmappable() spread about.
Maybe some helper to check whether or not we should unqueue the
defferred_list, but it out of scope in this patch, and maybe not worth it
There are many ways to play it. I had also considered doing it Zi Yan's
way, freezing always in the !mapping case as well as in the mapping case:
what overhead it adds would probably get lost amidst all the other overhead
of page migration. It will not be surprising if changes come later requiring
us always to freeze in the anon !swapcache case too, it always seemed a bit
surprising not to need freezing there. But for now I decided it's best to
keep the freezing to the case where it's known to be needed (but without
getting into __s).
Many ways to play it, and I've no objection if someone then changes it
around later; but we've no need to depart from what Andrew already has.
Except, he did ask one of us to send along the -fix removing the unnecessary
checks before its second folio_undo_large_rmappable() once your refactor
patch goes in: here it is below.
OK, let's keep it simple, thank your for pushing it out.
Grabbed, thanks.
[I guess this is the wrong place to say so, but folio_undo_large_rmappable()
is a dreadful name: it completely obscures what the function actually does,
and gives the false impression that the folio would be !large_rmappable
afterwards. I hope that one day the name gets changed to something like
folio_unqueue_deferred_split() or folio_cancel_deferred_split().]
Naming is important, but so also is commentary.
folio_undo_large_rmappable() lacks any.
[PATCH] mm: refactor folio_undo_large_rmappable() fix
Now that folio_undo_large_rmappable() is an inline function checking
order and large_rmappable for itself (and __folio_undo_large_rmappable()
is now declared even when CONFIG_TRANASPARENT_HUGEPAGE is off) there is
no need for folio_migrate_mapping() to check large and large_rmappable
first (in the mapping case when it has had to freeze anyway).
...
For folding in to mm-unstable's "mm: refactor folio_undo_large_rmappable()",
unless I'm too late and it's already mm-stable (no problem, just a cleanup).
Missed the mm-stable mergification by >that much<. I'll queue it
separately, thanks.
