All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@suse.de>
To: Minchan Kim <minchan@kernel.org>
Cc: "Huang, Ying" <ying.huang@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Hugh Dickins <hughd@google.com>,
	"Paul E . McKenney" <paulmck@linux.vnet.ibm.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Tim Chen <tim.c.chen@linux.intel.com>, Shaohua Li <shli@fb.com>,
	Mel Gorman <mgorman@techsingularity.net>,
	J???r???me Glisse <jglisse@redhat.com>,
	Michal Hocko <mhocko@suse.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	David Rientjes <rientjes@google.com>,
	Rik van Riel <riel@redhat.com>, Jan Kara <jack@suse.cz>,
	Dave Jiang <dave.jiang@intel.com>, Aaron Lu <aaron.lu@intel.com>
Subject: Re: [PATCH -V4 -mm] mm, swap: Fix race between swapoff and some swap operations
Date: Tue, 2 Jan 2018 10:21:03 +0000	[thread overview]
Message-ID: <20180102102103.mpah2ehglufwhzle@suse.de> (raw)
In-Reply-To: <20171223013653.GB5279@bgram>

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.

Hopefully that helps while I go back to the TODO mountain.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Mel Gorman <mgorman@suse.de>
To: Minchan Kim <minchan@kernel.org>
Cc: "Huang, Ying" <ying.huang@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Hugh Dickins <hughd@google.com>,
	"Paul E . McKenney" <paulmck@linux.vnet.ibm.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Tim Chen <tim.c.chen@linux.intel.com>, Shaohua Li <shli@fb.com>,
	Mel Gorman <mgorman@techsingularity.net>,
	J???r???me Glisse <jglisse@redhat.com>,
	Michal Hocko <mhocko@suse.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	David Rientjes <rientjes@google.com>,
	Rik van Riel <riel@redhat.com>, Jan Kara <jack@suse.cz>,
	Dave Jiang <dave.jiang@intel.com>, Aaron Lu <aaron.lu@intel.com>
Subject: Re: [PATCH -V4 -mm] mm, swap: Fix race between swapoff and some swap operations
Date: Tue, 2 Jan 2018 10:21:03 +0000	[thread overview]
Message-ID: <20180102102103.mpah2ehglufwhzle@suse.de> (raw)
In-Reply-To: <20171223013653.GB5279@bgram>

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.

Hopefully that helps while I go back to the TODO mountain.

-- 
Mel Gorman
SUSE Labs

  parent reply	other threads:[~2018-01-02 10:21 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-20  1:26 [PATCH -V4 -mm] mm, swap: Fix race between swapoff and some swap operations Huang, Ying
2017-12-20  1:26 ` Huang, Ying
2017-12-21  2:16 ` Minchan Kim
2017-12-21  2:16   ` Minchan Kim
2017-12-21  7:48   ` Huang, Ying
2017-12-21 23:58     ` Minchan Kim
2017-12-21 23:58       ` Minchan Kim
2017-12-22 14:14       ` Huang, Ying
2017-12-22 14:14         ` Huang, Ying
2017-12-22 16:14         ` Paul E. McKenney
2017-12-22 16:14           ` Paul E. McKenney
2017-12-25  1:28           ` Huang, Ying
2017-12-25  1:28             ` Huang, Ying
2017-12-23  1:36         ` Minchan Kim
2017-12-23  1:36           ` Minchan Kim
2017-12-26  5:33           ` Huang, Ying
2017-12-26  5:33             ` Huang, Ying
2018-01-02 10:21           ` Mel Gorman [this message]
2018-01-02 10:21             ` Mel Gorman
2018-01-02 11:29             ` Jan Kara
2018-01-02 11:29               ` Jan Kara
2018-01-02 13:29               ` Mel Gorman
2018-01-02 13:29                 ` Mel Gorman
2018-01-03  0:42                 ` Huang, Ying
2018-01-03  0:42                   ` Huang, Ying
2018-01-03  9:54                   ` Mel Gorman
2018-01-03  9:54                     ` Mel Gorman
2018-01-04  1:17                     ` Huang, Ying
2018-01-04  1:17                       ` Huang, Ying
2018-01-04 10:21                       ` Mel Gorman
2018-01-04 10:21                         ` Mel Gorman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180102102103.mpah2ehglufwhzle@suse.de \
    --to=mgorman@suse.de \
    --cc=aarcange@redhat.com \
    --cc=aaron.lu@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=dave.jiang@intel.com \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=jack@suse.cz \
    --cc=jglisse@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@suse.com \
    --cc=minchan@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=riel@redhat.com \
    --cc=rientjes@google.com \
    --cc=shli@fb.com \
    --cc=tim.c.chen@linux.intel.com \
    --cc=ying.huang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.