[...] >>> + >>> + if (!folio_trylock(folio)) >>> + continue; >> >> This is still wrong. This should all be protected by the "if >> (folio_test_swapcache(folio) || folio_test_dirty(folio))" as it was previously >> so that you only call folio_trylock() if that condition is true. You are >> unconditionally locking here, then unlocking, then relocking below if the >> condition is met. Just put everything inside the condition and lock once. > > I'm not sure if it's safe to call folio_mapcount() without holding the > folio lock. > > As mentioned earlier by David in the v2[1] >> What could work for large folios is making sure that #ptes that map the >> folio here correspond to the folio_mapcount(). And folio_mapcount() >> should be called under folio lock, to avoid racing with swapout/migration. > > [1] https://lore.kernel.org/all/5cc05529-eb80-410e-bc26-233b0ba0b21f@xxxxxxxxxx/ But I'm not suggesting that you should call folio_mapcount() without the lock. I'm proposing this: if (folio_test_swapcache(folio) || folio_test_dirty(folio)) { if (!folio_trylock(folio)) continue; /* - * If folio is shared with others, we mustn't clear - * the folio's dirty flag. + * If we have a large folio at this point, we know it is + * fully mapped so if its mapcount is the same as its + * number of pages, it must be exclusive. */ - if (folio_mapcount(folio) != 1) { + if (folio_mapcount(folio) != folio_nr_pages(folio)) { folio_unlock(folio); continue; } What am I missing?
