Re: [PATCH] mm: Make swapoff interruptible when unusing mms/shmem

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 10/1/26 7:17 AM, Chris Down wrote:
> try_to_unuse() only checks for a pending signal between mms, and
> shmem_unuse() doesn't check at all. That means that once swapoff gets to
> a process or a shmem file with a lot swapped out, nothing can interrupt
> it until every last page of it has been read back in.
> 
> Just as one example of where this can concretely show up, freezing tasks
> for suspend or hibernation has to wait for swapoff to notice the
> freezer's fake signal, and gives up after freeze_timeout_msecs (20
> seconds by default).
> 
> Here's a facetious example where one swaps out 2GiB of one process to a
> swap file on ext4, starts swapoff, and half a second later tries to
> freeze with pm_test=freezer. Writing to /sys/power/state then fails with
> EBUSY and this in dmesg:
> 
>      Freezing user space processes failed after 20.003 seconds (1 tasks refusing to freeze, wq_busy=0):
>      task:swapoff         state:D stack:0     pid:3175  tgid:3175  ppid:2955   task_flags:0x400100 flags:0x00000419
>      Call trace:
>       [...]
>       io_schedule+0x44/0x70
>       folio_wait_bit_common+0x1ec/0x3d0
>       __folio_lock+0x24/0x40
>       unuse_pte_range+0x2d0/0x348
>       unuse_vma+0x158/0x248
>       unuse_mm+0xfc/0x150
>       try_to_unuse+0x104/0x3f8
>       __do_sys_swapoff+0x220/0x5d8
>       [...]
> 
> The same goes for anything else that wants swapoff to stop, like an
> admin hitting ^C in a panic, of course.
> 
> Prior to commit b56a2d8af914 ("mm: rid swapoff of quadratic complexity")
> try_to_unuse() was driven by find_next_to_unuse() which checks for a
> signal before every entry, so let's restore that behaviour.
> 
> Just as an example of the improvements, here's how long freezing takes
> in the same test while swapoff is happening on my computer:
> 
>                        before                  after
>      400MiB anon       8.925s                  0.028s
>      400MiB shmem      1.639s                  0.003s
>      2GiB anon         failed after 20.003s    0.011s
> 
> Fixes: b56a2d8af914 ("mm: rid swapoff of quadratic complexity")
> Signed-off-by: Chris Down <chris@chrisdown.name>
> ---

Make sense to me. But ...

>   mm/shmem.c    | 4 ++++
>   mm/swapfile.c | 2 ++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index ae08cff4500c..72c8a61db76f 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1742,6 +1742,10 @@ static int shmem_unuse_inode(struct inode *inode, unsigned int type)
>   		if (ret < 0)
>   			break;
>   
> +		if (signal_pending(current)) {
> +			ret = -EINTR;
> +			break;
> +		}
>   		start = indices[folio_batch_count(&fbatch) - 1];
>   	} while (true);
>   
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 254ce86fa923..c3288910b3e3 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -2689,6 +2689,8 @@ static inline int unuse_pmd_range(struct vm_area_struct *vma, pud_t *pud,
>   	pmd = pmd_offset(pud, addr);
>   	do {
>   		cond_resched();
> +		if (signal_pending(current))
> +			return -EINTR;

Should we return -ERESTARTSYS instead based on the similar issue 
discussed in the following patch?

https://lore.kernel.org/all/20260720044103.905191-1-richardycc@google.com/

>   		next = pmd_addr_end(addr, end);
>   		ret = unuse_pte_range(vma, pmd, addr, next, type);
>   		if (ret)
> 
> base-commit: 2ddb90ee544ae97215afc4698dc223293997cc43