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. > > 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. 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.
