Re: Whether is the race for SWP_SYNCHRONOUS_IO possible? (was Re: [PATCH v3 6/7] mm/swap, shmem: use unified swapin helper for shmem)

From: Chris Li
Date: Wed Jan 31 2024 - 18:43:11 EST


On Tue, Jan 30, 2024 at 6:53 PM Huang, Ying <ying.huang@xxxxxxxxx> wrote:
>
> Hi, Minchan,
>
> When I review the patchset from Kairui, I checked the code to skip swap
> cache in do_swap_page() for swap device with SWP_SYNCHRONOUS_IO. Is the
> following race possible? Where a page is swapped out to a swap device
> with SWP_SYNCHRONOUS_IO and the swap count is 1. Then 2 threads of the
> process runs on CPU0 and CPU1 as below. CPU0 is running do_swap_page().
>
> CPU0 CPU1
> ---- ----
> swap_cache_get_folio()
> check sync io and swap count
> alloc folio
> swap_readpage()
> folio_lock_or_retry()
> swap in the swap entry
> write page
> swap out to same swap entry
> pte_offset_map_lock()
> check pte_same()
> swap_free() <-- new content lost!
> set_pte_at() <-- stale page!
> folio_unlock()
> pte_unmap_unlock()

Yes, that path looks possible but hard to hit due to the requirement
of swap in and swap out in a short window.
I have the similar question on the previous zswap rb tree to xarray
discussion regarding deleting an entry where the entry might change
due to swap in then swap out.

Chris