On Thu, Jul 11, 2024 at 1:53 PM David Hildenbrand <david@xxxxxxxxxx> wrote: > > On 11.07.24 21:49, Yu Zhao wrote: > > On Thu, Jul 11, 2024 at 1:20 PM David Hildenbrand <david@xxxxxxxxxx> wrote: > >> > >> On 11.07.24 21:18, David Hildenbrand wrote: > >>> On 11.07.24 20:56, David Hildenbrand wrote: > >>>> On 11.07.24 20:54, Jason A. Donenfeld wrote: > >>>>> On Thu, Jul 11, 2024 at 08:24:07PM +0200, David Hildenbrand wrote: > >>>>>>> And PG_large_rmappable seems to only be used for hugetlb branches. > >>>>>> > >>>>>> It should be set for THP/large folios. > >>>>> > >>>>> And it's tested too, apparently. > >>>>> > >>>>> Okay, well, how disappointing is this below? Because I'm running out of > >>>>> tricks for flag reuse. > >>>>> > >>>>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > >>>>> index b9e914e1face..c1ea49a7f198 100644 > >>>>> --- a/include/linux/page-flags.h > >>>>> +++ b/include/linux/page-flags.h > >>>>> @@ -110,6 +110,7 @@ enum pageflags { > >>>>> PG_workingset, > >>>>> PG_error, > >>>>> PG_owner_priv_1, /* Owner use. If pagecache, fs may use*/ > >>>>> + PG_owner_priv_2, > >>>> > >>>> Oh no, no new page flags please :) > >>>> > >>>> Maybe just follow what Linux suggested: pass vma to pte_dirty() and > >>>> always return false for these special VMAs. > >>> > >>> ... or look into removing that one case that gives us headake. > >>> > >>> No idea what would happen if we do the following: > >>> > >>> CCing Yu Zhao. > >>> > >>> diff --git a/mm/vmscan.c b/mm/vmscan.c > >>> index 0761f91b407f..d1dfbd4fd38d 100644 > >>> --- a/mm/vmscan.c > >>> +++ b/mm/vmscan.c > >>> @@ -4280,14 +4280,9 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c > >>> return true; > >>> } > >>> > >>> - /* dirty lazyfree */ > >>> - if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio)) { > >>> - success = lru_gen_del_folio(lruvec, folio, true); > >>> - VM_WARN_ON_ONCE_FOLIO(!success, folio); > >>> - folio_set_swapbacked(folio); > >>> - lruvec_add_folio_tail(lruvec, folio); > >>> - return true; > >>> - } > >>> + /* lazyfree: we may not be allowed to set swapbacked: MAP_DROPPABLE */ > >>> + if (type == LRU_GEN_FILE && folio_test_anon(folio) && folio_test_dirty(folio)) > >>> + return false; > > > > This is an optimization to avoid an unnecessary trip to > > shrink_folio_list(), so it's safe to delete the entire 'if' block, and > > that would be preferable than leaving a dangling 'if'. > > Great, thanks. > > > > >> Note that something is unclear to me: are we maybe running into that > >> code also if folio_set_swapbacked() is already set and we are not in the > >> lazyfree path (in contrast to what is documented)? > > > > Not sure what you mean: either rmap sees pte_dirty() and does > > folio_mark_dirty() and then folio_set_swapbacked(); or MGLRU does the > > same sequence, with the first two steps in walk_pte_range() and the > > last one here. > > Let me rephrase: > > Checking for lazyfree is > > "folio_test_anon(folio) && !folio_test_swapbacked(folio)" > > Testing for dirtied lazyfree is > > "folio_test_anon(folio) && !folio_test_swapbacked(folio) && > folio_test)dirty(folio)" > > So I'm wondering about the missing folio_test_swapbacked() test. It's not missing: type == LRU_GEN_FILE means folio_is_file_lru(), which in turn means !folio_test_swapbacked().