From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752816AbeABLaB (ORCPT + 1 other); Tue, 2 Jan 2018 06:30:01 -0500 Received: from mx2.suse.de ([195.135.220.15]:41590 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751872AbeABLaA (ORCPT ); Tue, 2 Jan 2018 06:30:00 -0500 Date: Tue, 2 Jan 2018 12:29:55 +0100 From: Jan Kara To: Mel Gorman Cc: Minchan Kim , "Huang, Ying" , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Hugh Dickins , "Paul E . McKenney" , Johannes Weiner , Tim Chen , Shaohua Li , Mel Gorman , J???r???me Glisse , Michal Hocko , Andrea Arcangeli , David Rientjes , Rik van Riel , Jan Kara , Dave Jiang , Aaron Lu Subject: Re: [PATCH -V4 -mm] mm, swap: Fix race between swapoff and some swap operations Message-ID: <20180102112955.GA29170@quack2.suse.cz> References: <20171220012632.26840-1-ying.huang@intel.com> <20171221021619.GA27475@bbox> <871sjopllj.fsf@yhuang-dev.intel.com> <20171221235813.GA29033@bbox> <87r2rmj1d8.fsf@yhuang-dev.intel.com> <20171223013653.GB5279@bgram> <20180102102103.mpah2ehglufwhzle@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180102102103.mpah2ehglufwhzle@suse.de> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Tue 02-01-18 10:21:03, Mel Gorman wrote: > On Sat, Dec 23, 2017 at 10:36:53AM +0900, Minchan Kim wrote: > > > code path. It appears that similar situation is possible for them too. > > > > > > The file cache pages will be delete from file cache address_space before > > > address_space (embedded in inode) is freed. But they will be deleted > > > from LRU list only when its refcount dropped to zero, please take a look > > > at put_page() and release_pages(). While address_space will be freed > > > after putting reference to all file cache pages. If someone holds a > > > reference to a file cache page for quite long time, it is possible for a > > > file cache page to be in LRU list after the inode/address_space is > > > freed. > > > > > > And I found inode/address_space is freed witch call_rcu(). I don't know > > > whether this is related to page_mapping(). > > > > > > This is just my understanding. > > > > Hmm, it smells like a bug of __isolate_lru_page. > > > > Ccing Mel: > > > > What locks protects address_space destroying when race happens between > > inode trauncation and __isolate_lru_page? > > > > I'm just back online and have a lot of catching up to do so this is a rushed > answer and I didn't read the background of this. However the question is > somewhat ambiguous and the scope is broad as I'm not sure which race you > refer to. For file cache pages, I wouldnt' expect the address_space to be > destroyed specifically as long as the inode exists which is the structure > containing the address_space in this case. A page on the LRU being isolated > in __isolate_lru_page will have an elevated reference count which will > pin the inode until remove_mapping is called which holds the page lock > while inode truncation looking at a page for truncation also only checks > page_mapping under the page lock. Very broadly speaking, pages avoid being > added back to an inode being freed by checking the I_FREEING state. So I'm wondering what prevents the following: CPU1 CPU2 truncate(inode) __isolate_lru_page() ... truncate_inode_page(mapping, page); delete_from_page_cache(page) spin_lock_irqsave(&mapping->tree_lock, flags); __delete_from_page_cache(page, NULL) page_cache_tree_delete(..) ... mapping = page_mapping(page); page->mapping = NULL; ... spin_unlock_irqrestore(&mapping->tree_lock, flags); page_cache_free_page(mapping, page) put_page(page) if (put_page_testzero(page)) -> false - inode now has no pages and can be freed including embedded address_space if (mapping && !mapping->a_ops->migratepage) - we've dereferenced mapping which is potentially already free. This all seems very theoretical but in principle possible... Honza -- Jan Kara SUSE Labs, CR