Re: [PATCH] mm: fix the race between swapin_readahead and SWP_SYNCHRONOUS_IO path

[Minchan Kim <minchan@kernel.org>]

Hi Vinayak,

On Fri, Sep 13, 2019 at 02:35:41PM +0530, Vinayak Menon wrote:
> 
> On 9/12/2019 10:44 PM, Minchan Kim wrote:
> > Hi Vinayak,
> >
> > On Wed, Sep 11, 2019 at 03:37:23PM +0530, Vinayak Menon wrote:
> >
> > < snip >
> >
> >>>> Can swapcache check be done like below, before taking the SWP_SYNCHRONOUS_IO path, as an alternative ?
> >>> With your approach, what prevent below scenario?
> >>>
> >>> A                                                       B
> >>>
> >>>                                             do_swap_page
> >>>                                             SWP_SYNCHRONOUS_IO && __swap_count == 1
> >>
> >> As shrink_page_list is picking the page from LRU and B is trying to read from swap simultaneously, I assume someone had read
> >>
> >> the page from swap prior to B, when its swap_count was say 2 (for it to be reclaimed by shrink_page_list now)
> > It could happen after B saw __swap_count == 1. Think about forking new process.
> > In that case, swap_count is 2 and the forked process will access the page(it
> > ends up freeing zram slot but the page would be swap cache. However, B process
> > doesn't know it).
> 
> 
> Okay, so when B has read __swap_count == 1, it means that it has taken down_read on mmap_sem in fault path
> 
> already. This means fork will not be able to proceed which needs to have down_write on parent's mmap_sem ?
> 

You are exactly right. However, I still believe better option to solve
the issue is to check swap_count and delte only if swap_count == 1
in swap_slot_free_notify because it's zram specific issue and more safe
without depending other lock scheme.