On 9/17/2019 1:35 AM, Minchan Kim wrote: > 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. Sure. Let me know if you want me to post a patch for that.
