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.
