From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751440AbeAWGNq (ORCPT ); Tue, 23 Jan 2018 01:13:46 -0500 Received: from mail-io0-f172.google.com ([209.85.223.172]:41557 "EHLO mail-io0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751213AbeAWGNp (ORCPT ); Tue, 23 Jan 2018 01:13:45 -0500 X-Google-Smtp-Source: AH8x227/qPlcW5Oh6+LcVn3FFBUKa47XE+riuwaNExDBR35sjapB92da9/rD5QwOT5DoN62ldQga0Q== Date: Tue, 23 Jan 2018 15:13:38 +0900 From: Minchan Kim To: Mel Gorman Cc: Andrew Morton , "Huang, Ying" , Jan Kara , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm: Pin address_space before dereferencing it while isolating an LRU page Message-ID: <20180123061338.cpsmhih2cvbo4gr6@bbox-2.seo.corp.google.com> References: <20180104102512.2qos3h5vqzeisrek@techsingularity.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180104102512.2qos3h5vqzeisrek@techsingularity.net> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 04, 2018 at 10:25:12AM +0000, Mel Gorman wrote: > Minchan Kim asked the following question -- what locks protects > address_space destroying when race happens between inode trauncation and > __isolate_lru_page? Jan Kara clarified by describing the race as follows > > 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. > > The race is theoritically possible but unlikely. Before the > delete_from_page_cache, truncate_cleanup_page is called so the page is > likely to be !PageDirty or PageWriteback which gets skipped by the only > caller that checks the mappping in __isolate_lru_page. Even if the race > occurs, a substantial amount of work has to happen during a tiny window > with no preemption but it could potentially be done using a virtual machine > to artifically slow one CPU or halt it during the critical window. > > This patch should eliminate the race with truncation by try-locking the page > before derefencing mapping and aborting if the lock was not acquired. There > was a suggestion from Huang Ying to use RCU as a side-effect to prevent > mapping being freed. However, I do not like the solution as it's an > unconventional means of preserving a mapping and it's not a context where > rcu_read_lock is obviously protecting rcu data. > > Fixes: c82449352854 ("mm: compaction: make isolate_lru_page() filter-aware again") > Signed-off-by: Mel Gorman Acked-by: Minchan Kim Thanks for the patch.