Re: [PATCH 2/3] mm/swap: account synchronous swap reads as memory stall

[Kairui Song]

On Fri, Oct 30, 2026 at 5:00 AM Johannes Weiner <hannes@xxxxxxxxxxx> wrote:
>
> On Fri, Oct 30, 2026 at 02:31:42AM +0800, Kairui Song via B4 Relay wrote:
> > From: Josef Bacik <josef@xxxxxxxxxxxxxx>
> >
> > Swap-ins from SWP_SYNCHRONOUS_IO devices such as zram are no longer
> > counted in PSI memory pressure or in the swapin and thrashing delays of
> > delay accounting.  Refaults of the workingset from zram, the main input
> > to userspace OOM handlers like lmkd and systemd-oomd on zram systems,
> > become invisible in /proc/pressure/memory.
> >
> > swap_read_folio() opens the accounting window with
> > psi_memstall_enter() and delayacct_swapin_start() and, per its comment,
> > means to count the submission time.  For synchronous devices the
> > submission is the entire read: swap_bdev_submit_read() does
> > submit_bio_wait() and completes the folio.  Since reads are batched in
> > a swap_iocb, swap_read_folio() only queues the folio with
> > swap_add_folio(), and the bio is submitted later by swap_read_submit()
> > in the caller, e.g. swapin_sync(), after the window has been closed.
> > By then the folio is uptodate and unlocked, so the fault does not wait
> > on the folio lock either and the read time is not counted anywhere.
> >
> > Found by code inspection; not reproduced.
> >
> > Fix by submitting synchronous reads from within swap_read_folio(), as
> > was done before the batching, rather than from the caller.  This gives
> > up batching of readahead for synchronous devices, which do their I/O
> > inline anyway, in exchange for keeping the accounting simple and in one
> > place.
> >
> > Fixes: dda8fb68b590 ("mm/swap: also use struct swap_iocb for block I/O")
> > Assisted-by: LLM
> > Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> > Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> > Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
>
> Hm, this isn't a complete fix to the problem described. IO throttling
> happens at submission time, so there can be significant delays on
> async devices too that are no longer tracked since the culprit change.

Yeah you are right! Thanks for the heads up, not only a sync isssue
indeed, async devices are affected too :(, throttled swap fault also
went unaccounted.

> How about keeping the accounting window open across the batch? Opens
> on first read, closes on submission. A batch with at least one
> workingset folio is "contaminated" and the whole sequence and
> submission becomes a stall. Would need to pull the !can_merge flush
> out of the batch-add function to get the accounting window right but
> that seems reasonable.

Won't this cause an amplified PSI reading if only one folio in a batch
is workingset? Maybe not a big issue though.

>
> Something like this:
>
> diff --git a/include/linux/swap_ops.h b/include/linux/swap_ops.h
> index 57ac6c703f68..6e45c9204482 100644
> --- a/include/linux/swap_ops.h
> +++ b/include/linux/swap_ops.h
> @@ -17,6 +17,10 @@ struct swap_iocb {
>  struct swap_io_ctx {
>         struct swap_iocb        *sio;
>         struct swap_info_struct *sis;
> +       bool                    in_batch;
> +       bool                    workingset;
> +       bool                    in_thrashing;
> +       unsigned long           in_memstall;
>  };
>
>  /*
> diff --git a/mm/page_io.c b/mm/page_io.c
> index c6824fcd483e..22e5ac1d298f 100644
> --- a/mm/page_io.c
> +++ b/mm/page_io.c
> @@ -197,6 +197,19 @@ static void swap_zeromap_folio_clear(struct folio *folio)
>         swap_cluster_unlock(ci);
>  }
>
> +static bool swap_can_merge(struct swap_io_ctx *ctx, struct folio *folio,
> +               int rw)
> +{
> +       struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);
> +       struct bio_vec *last_bv = &ctx->sio->bvecs[ctx->sio->nr_bvecs - 1];
> +       struct folio *prev_folio = bvec_folio(last_bv);
> +       size_t prev_folio_size = folio_size(prev_folio);
> +
> +       if (ctx->sis != sis)
> +               return false;
> +       return sis->ops->can_merge(folio, prev_folio, prev_folio_size, rw);
> +}
> +
>  /*
>   * We may have stale swap cache pages in memory: notice
>   * them here and get rid of the unnecessary final write.
> @@ -218,6 +231,13 @@ int swap_writeout(struct swap_io_ctx *ctx, struct folio *folio)
>                 goto out_unlock;
>         }
>
> +       /*
> +        * Don't hold up a batch this folio can't join for the zero
> +        * check and possible compression. Submit it now.
> +        */
> +       if (ctx->sio && !swap_can_merge(ctx, folio, WRITE))
> +               swap_write_submit(ctx);
> +
>         /*
>          * Use the swap table zero mark to avoid doing IO for zero-filled
>          * folios. The zero mark is protected by the cluster lock, which is
> @@ -322,32 +342,15 @@ int sio_pool_init(void)
>         return 0;
>  }
>
> -static bool swap_can_merge(struct swap_io_ctx *ctx, struct folio *folio,
> -               int rw)
> -{
> -       struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);
> -       struct bio_vec *last_bv = &ctx->sio->bvecs[ctx->sio->nr_bvecs - 1];
> -       struct folio *prev_folio = bvec_folio(last_bv);
> -       size_t prev_folio_size = folio_size(prev_folio);
> -
> -       if (ctx->sis != sis)
> -               return false;
> -       return sis->ops->can_merge(folio, prev_folio, prev_folio_size, rw);
> -}
> -
> +/*
> + * Callers submit a pending batch that @folio can't join before adding
> + * it, see swap_writeout() and swap_read_folio().
> + */
>  static void swap_add_folio(struct swap_io_ctx *ctx, struct folio *folio, int rw)
>  {
>         struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);
>         struct swap_iocb *sio = ctx->sio;
>
> -       if (sio && !swap_can_merge(ctx, folio, rw)) {
> -               if (rw == WRITE)
> -                       swap_write_submit(ctx);
> -               else
> -                       swap_read_submit(ctx);
> -               sio = ctx->sio;
> -       }
> -
>         if (!sio) {
>                 ctx->sis = sis;
>                 ctx->sio = sio = mempool_alloc(sio_pool, GFP_NOIO);
> @@ -460,42 +463,45 @@ void swap_read_folio(struct swap_io_ctx *ctx, struct folio *folio)
>         struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);
>         bool synchronous = sis->flags & SWP_SYNCHRONOUS_IO;
>         bool workingset = folio_test_workingset(folio);
> -       unsigned long pflags;
> -       bool in_thrashing;
>
>         VM_BUG_ON_FOLIO(!folio_test_swapcache(folio) && !synchronous, folio);
>         VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
>         VM_BUG_ON_FOLIO(folio_test_uptodate(folio), folio);
>
> +       /*
> +        * Don't hold up a batch this folio can't join for its lookup
> +        * and possible decompression. Submit it now.
> +        */
> +       if (ctx->sio && !swap_can_merge(ctx, folio, READ))
> +               swap_read_submit(ctx);
> +
>         /*
>          * Count submission time as memory stall and delay. When the device
>          * is congested, or the submitting cgroup IO-throttled, submission
> -        * can be a significant part of overall IO time.
> +        * can be a significant part of overall IO time. Reads are batched
> +        * and submitted later, so the windows close in swap_read_submit().
>          */
> -       if (workingset) {
> -               delayacct_thrashing_start(&in_thrashing);
> -               psi_memstall_enter(&pflags);
> +       if (!ctx->in_batch) {
> +               delayacct_swapin_start();
> +               ctx->in_batch = true;
> +       }
> +       if (workingset && !ctx->workingset) {
> +               delayacct_thrashing_start(&ctx->in_thrashing);
> +               psi_memstall_enter(&ctx->in_memstall);
> +               ctx->workingset = true;
>         }
> -       delayacct_swapin_start();
>
>         if (swap_read_folio_zeromap(folio)) {
>                 folio_unlock(folio);
> -               goto finish;
> +               return;
>         }
>
>         if (zswap_load(folio) != -ENOENT)
> -               goto finish;
> +               return;
>
>         /* We have to read from slower devices. Increase zswap protection. */
>         zswap_folio_swapin(folio);
>         swap_add_folio(ctx, folio, READ);
> -
> -finish:
> -       if (workingset) {
> -               delayacct_thrashing_end(&in_thrashing);
> -               psi_memstall_leave(&pflags);
> -       }
> -       delayacct_swapin_end();
>  }
>
>  static void swap_write_end(struct swap_iocb *sio, bool failed)
> @@ -709,9 +715,19 @@ void swap_write_submit(struct swap_io_ctx *ctx)
>  void swap_read_submit(struct swap_io_ctx *ctx)
>  {
>         if (!ctx->sio)
> -               return;
> +               goto out;
>         count_vm_events(NRSWPIN, 1);
>         ctx->sis->ops->submit_read(ctx);
>         ctx->sio = NULL;
>         ctx->sis = NULL;
> +out:
> +       if (ctx->workingset) {
> +               delayacct_thrashing_end(&ctx->in_thrashing);
> +               psi_memstall_leave(&ctx->in_memstall);
> +               ctx->workingset = false;
> +       }
> +       if (ctx->in_batch) {
> +               delayacct_swapin_end();
> +               ctx->in_batch = false;
> +       }
>  }

Nice suggestion, and I forgot to Cc Christoph on this though,
worth getting more attention.