On Tue, Jun 11, 2024 at 2:45 PM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote: > > Zswap does not support storing or loading large folios. Until proper > support is added, attempts to load large folios from zswap are a bug. > > For example, if a swapin fault observes that contiguous PTEs are > pointing to contiguous swap entries and tries to swap them in as a large > folio, swap_read_folio() will pass in a large folio to zswap_load(), but > zswap_load() will only effectively load the first page in the folio. If > the first page is not in zswap, the folio will be read from disk, even > though other pages may be in zswap. > > In both cases, this will lead to silent data corruption. Proper support > needs to be added before large folio swapins and zswap can work > together. > > Looking at callers of swap_read_folio(), it seems like they are either > allocated from __read_swap_cache_async() or do_swap_page() in the > SWP_SYNCHRONOUS_IO path. Both of which allocate order-0 folios, so > everything is fine for now. > > However, there is ongoing work to add to support large folio swapins > [1]. To make sure new development does not break zswap (or get broken by > zswap), add minimal handling of incorrect loads of large folios to > zswap. > > First, move the call folio_mark_uptodate() inside zswap_load(). > > If a large folio load is attempted, and zswap was ever enabled on the > system, return 'true' without calling folio_mark_uptodate(). This will > prevent the folio from being read from disk, and will emit an IO error > because the folio is not uptodate (e.g. do_swap_fault() will return > VM_FAULT_SIGBUS). It may not be reliable recovery in all cases, but it > is better than nothing. > > This was tested by hacking the allocation in __read_swap_cache_async() > to use order 2 and __GFP_COMP. > > In the future, to handle this correctly, the swapin code should: > (a) Fallback to order-0 swapins if zswap was ever used on the machine, > because compressed pages remain in zswap after it is disabled. > (b) Add proper support to swapin large folios from zswap (fully or > partially). > > Probably start with (a) then followup with (b). > > [1]https://lore.kernel.org/linux-mm/20240304081348.197341-6-21cnbao@xxxxxxxxx/ > > Signed-off-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx> Acked-by: Barry Song <baohua@xxxxxxxxxx> > --- > mm/page_io.c | 1 - > mm/zswap.c | 12 ++++++++++++ > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/mm/page_io.c b/mm/page_io.c > index f1a9cfab6e748..8f441dd8e109f 100644 > --- a/mm/page_io.c > +++ b/mm/page_io.c > @@ -517,7 +517,6 @@ void swap_read_folio(struct folio *folio, struct swap_iocb **plug) > delayacct_swapin_start(); > > if (zswap_load(folio)) { > - folio_mark_uptodate(folio); > folio_unlock(folio); > } else if (data_race(sis->flags & SWP_FS_OPS)) { > swap_read_folio_fs(folio, plug); > diff --git a/mm/zswap.c b/mm/zswap.c > index 7fcd751e847d6..505f4b9812891 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1566,6 +1566,17 @@ bool zswap_load(struct folio *folio) > if (zswap_never_enabled()) > return false; > > + /* > + * Large folios should not be swapped in while zswap is being used, as > + * they are not properly handled. Zswap does not properly load large > + * folios, and a large folio may only be partially in zswap. > + * > + * Return true without marking the folio uptodate so that an IO error is > + * emitted (e.g. do_swap_page() will sigbus). > + */ > + if (WARN_ON_ONCE(folio_test_large(folio))) > + return true; > + > /* > * When reading into the swapcache, invalidate our entry. The > * swapcache can be the authoritative owner of the page and > @@ -1600,6 +1611,7 @@ bool zswap_load(struct folio *folio) > folio_mark_dirty(folio); > } > > + folio_mark_uptodate(folio); > return true; > } > > -- > 2.45.2.505.gda0bf45e8d-goog >
