On Tue, Sep 17, 2019 at 11:08:49AM +0530, Vinayak Menon wrote: > > 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. > Please post a patch. Thanks, Vinayak!
