All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jerome Glisse <jglisse@redhat.com>
To: Hugh Dickins <hughd@google.com>
Cc: linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
	Huang Ying <ying.huang@intel.com>,
	linux-kernel@vger.kernel.org,
	Steven Capper <steve.capper@linaro.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Rabin Vincent <rabinv@axis.com>,
	linux-arm-kernel@lists.infradead.org,
	rmk+kernel@arm.linux.org.uk, Guo Ren <guoren@kernel.org>,
	linux-mips@vger.kernel.org, Ralf Baechle <ralf@linux-mips.org>,
	Paul Burton <paulburton@kernel.org>,
	James Hogan <jhogan@kernel.org>, Ley Foon Tan <lftan@altera.com>,
	nios2-dev@lists.rocketboards.org, linux-parisc@vger.kernel.org,
	Helge Deller <deller@gmx.de>,
	"James E.J. Bottomley" <James.Bottomley@hansenpartnership.com>,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	Rich Felker <dalias@libc.org>,
	linux-sh@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	sparclinux@vger.kernel.org, Guan Xuetao <gxt@pku.edu.cn>,
	linux-xtensa@linux-xtensa.org, Max Filippov <jcmvbkbc@gmail.com>,
	Chris Zankel <chris@zankel.net>
Subject: Re: Cache flush issue with page_mapping_file() and swap back shmem page ?
Date: Thu, 28 May 2020 07:20:25 -0400	[thread overview]
Message-ID: <20200528112025.GA10175@redhat.com> (raw)
In-Reply-To: <alpine.LSU.2.11.2005272021220.3857@eggly.anvils>

On Wed, May 27, 2020 at 08:46:22PM -0700, Hugh Dickins wrote:
> Hi Jerome,
> 
> On Wed, 27 May 2020, Jerome Glisse wrote:
> > So any arch code which uses page_mapping_file() might get the wrong
> > answer, this function will return NULL for a swap backed page which
> > can be a shmem pages. But shmem pages can still be shared among
> > multiple process (and possibly at different virtual addresses if
> > mremap was use).
> > 
> > Attached is a patch that changes page_mapping_file() to return the
> > shmem mapping for swap backed shmem page. I have not tested it (no
> > way for me to test all those architecture) and i spotted this while
> > working on something else. So i hope someone can take a closer look.
> 
> I'm certainly no expert on flush_dcache_page() and friends, but I'd
> be very surprised if such a problem exists, yet has gone unnoticed
> for so long.  page_mapping_file() itself is fairly new, added when
> a risk of crashing on a race with swapoff came in: but the previous
> use of page_mapping() would have suffered equally if there were such
> a cache flushinhg problem here.
> 
> And I'm afraid your patch won't do anything to help if there is a
> problem: very soon after shmem calls add_to_swap_cache(), it calls
> shmem_delete_from_page_cache(), which sets page->mapping to NULL.
> 
> But I can assure you that a shmem page (unlike an anon page) is never
> put into swap cache while it is mapped into userspace, and never
> mapped into userspace while it is still in swap cache: does that help?
> 

You are right i missed/forgot the part where shmem is never swapcache
and mapped at the same time, thus page_mapping_file() can return NULL
for those as they can no longer have alias mapping.

Thank you Hugh
Jérôme


WARNING: multiple messages have this Message-ID (diff)
From: Jerome Glisse <jglisse@redhat.com>
To: Hugh Dickins <hughd@google.com>
Cc: Rich Felker <dalias@libc.org>,
	linux-sh@vger.kernel.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	linux-mips@vger.kernel.org,
	"James E.J. Bottomley" <James.Bottomley@hansenpartnership.com>,
	linux-mm@kvack.org, Guo Ren <guoren@kernel.org>,
	sparclinux@vger.kernel.org, Paul Burton <paulburton@kernel.org>,
	Helge Deller <deller@gmx.de>, Huang Ying <ying.huang@intel.com>,
	James Hogan <jhogan@kernel.org>,
	rmk+kernel@arm.linux.org.uk, linux-xtensa@linux-xtensa.org,
	Steven Capper <steve.capper@linaro.org>,
	Rabin Vincent <rabinv@axis.com>, Ley Foon Tan <lftan@altera.com>,
	Guan Xuetao <gxt@pku.edu.cn>,
	linux-arm-kernel@lists.infradead.org,
	Chris Zankel <chris@zankel.net>,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	linux-parisc@vger.kernel.org, Max Filippov <jcmvbkbc@gmail.com>,
	linux-kernel@vger.kernel.org, Ralf Baechle <ralf@linux-mips.org>,
	nios2-dev@lists.rocketboards.org,
	Andrew Morton <akpm@linux-foundation.org>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: Cache flush issue with page_mapping_file() and swap back shmem page ?
Date: Thu, 28 May 2020 11:20:25 +0000	[thread overview]
Message-ID: <20200528112025.GA10175@redhat.com> (raw)
In-Reply-To: <alpine.LSU.2.11.2005272021220.3857@eggly.anvils>

On Wed, May 27, 2020 at 08:46:22PM -0700, Hugh Dickins wrote:
> Hi Jerome,
> 
> On Wed, 27 May 2020, Jerome Glisse wrote:
> > So any arch code which uses page_mapping_file() might get the wrong
> > answer, this function will return NULL for a swap backed page which
> > can be a shmem pages. But shmem pages can still be shared among
> > multiple process (and possibly at different virtual addresses if
> > mremap was use).
> > 
> > Attached is a patch that changes page_mapping_file() to return the
> > shmem mapping for swap backed shmem page. I have not tested it (no
> > way for me to test all those architecture) and i spotted this while
> > working on something else. So i hope someone can take a closer look.
> 
> I'm certainly no expert on flush_dcache_page() and friends, but I'd
> be very surprised if such a problem exists, yet has gone unnoticed
> for so long.  page_mapping_file() itself is fairly new, added when
> a risk of crashing on a race with swapoff came in: but the previous
> use of page_mapping() would have suffered equally if there were such
> a cache flushinhg problem here.
> 
> And I'm afraid your patch won't do anything to help if there is a
> problem: very soon after shmem calls add_to_swap_cache(), it calls
> shmem_delete_from_page_cache(), which sets page->mapping to NULL.
> 
> But I can assure you that a shmem page (unlike an anon page) is never
> put into swap cache while it is mapped into userspace, and never
> mapped into userspace while it is still in swap cache: does that help?
> 

You are right i missed/forgot the part where shmem is never swapcache
and mapped at the same time, thus page_mapping_file() can return NULL
for those as they can no longer have alias mapping.

Thank you Hugh
Jérôme

WARNING: multiple messages have this Message-ID (diff)
From: Jerome Glisse <jglisse@redhat.com>
To: Hugh Dickins <hughd@google.com>
Cc: Rich Felker <dalias@libc.org>,
	linux-sh@vger.kernel.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	linux-mips@vger.kernel.org,
	"James E.J. Bottomley" <James.Bottomley@hansenpartnership.com>,
	linux-mm@kvack.org, Guo Ren <guoren@kernel.org>,
	sparclinux@vger.kernel.org, Paul Burton <paulburton@kernel.org>,
	Helge Deller <deller@gmx.de>, Huang Ying <ying.huang@intel.com>,
	James Hogan <jhogan@kernel.org>,
	rmk+kernel@arm.linux.org.uk, linux-xtensa@linux-xtensa.org,
	Steven Capper <steve.capper@linaro.org>,
	Rabin Vincent <rabinv@axis.com>, Ley Foon Tan <lftan@altera.com>,
	Guan Xuetao <gxt@pku.edu.cn>,
	linux-arm-kernel@lists.infradead.org,
	Chris Zankel <chris@zankel.net>,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	linux-parisc@vger.kernel.org, Max Filippov <jcmvbkbc@gmail.com>,
	linux-kernel@vger.kernel.org, Ralf Baechle <ralf@linux-mips.org>,
	nios2-dev@lists.rocketboards.org,
	Andrew Morton <akpm@linux-foundation.org>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: Cache flush issue with page_mapping_file() and swap back shmem page ?
Date: Thu, 28 May 2020 07:20:25 -0400	[thread overview]
Message-ID: <20200528112025.GA10175@redhat.com> (raw)
In-Reply-To: <alpine.LSU.2.11.2005272021220.3857@eggly.anvils>

On Wed, May 27, 2020 at 08:46:22PM -0700, Hugh Dickins wrote:
> Hi Jerome,
> 
> On Wed, 27 May 2020, Jerome Glisse wrote:
> > So any arch code which uses page_mapping_file() might get the wrong
> > answer, this function will return NULL for a swap backed page which
> > can be a shmem pages. But shmem pages can still be shared among
> > multiple process (and possibly at different virtual addresses if
> > mremap was use).
> > 
> > Attached is a patch that changes page_mapping_file() to return the
> > shmem mapping for swap backed shmem page. I have not tested it (no
> > way for me to test all those architecture) and i spotted this while
> > working on something else. So i hope someone can take a closer look.
> 
> I'm certainly no expert on flush_dcache_page() and friends, but I'd
> be very surprised if such a problem exists, yet has gone unnoticed
> for so long.  page_mapping_file() itself is fairly new, added when
> a risk of crashing on a race with swapoff came in: but the previous
> use of page_mapping() would have suffered equally if there were such
> a cache flushinhg problem here.
> 
> And I'm afraid your patch won't do anything to help if there is a
> problem: very soon after shmem calls add_to_swap_cache(), it calls
> shmem_delete_from_page_cache(), which sets page->mapping to NULL.
> 
> But I can assure you that a shmem page (unlike an anon page) is never
> put into swap cache while it is mapped into userspace, and never
> mapped into userspace while it is still in swap cache: does that help?
> 

You are right i missed/forgot the part where shmem is never swapcache
and mapped at the same time, thus page_mapping_file() can return NULL
for those as they can no longer have alias mapping.

Thank you Hugh
Jérôme


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-05-28 11:20 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-28  0:20 Cache flush issue with page_mapping_file() and swap back shmem page ? Jerome Glisse
2020-05-28  0:20 ` Jerome Glisse
2020-05-28  0:20 ` Jerome Glisse
2020-05-28  3:46 ` Hugh Dickins
2020-05-28  3:46   ` Hugh Dickins
2020-05-28  3:46   ` Hugh Dickins
2020-05-28 11:20   ` Jerome Glisse [this message]
2020-05-28 11:20     ` Jerome Glisse
2020-05-28 11:20     ` Jerome Glisse

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=20200528112025.GA10175@redhat.com \
    --to=jglisse@redhat.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=akpm@linux-foundation.org \
    --cc=catalin.marinas@arm.com \
    --cc=chris@zankel.net \
    --cc=dalias@libc.org \
    --cc=davem@davemloft.net \
    --cc=deller@gmx.de \
    --cc=guoren@kernel.org \
    --cc=gxt@pku.edu.cn \
    --cc=hughd@google.com \
    --cc=jcmvbkbc@gmail.com \
    --cc=jhogan@kernel.org \
    --cc=lftan@altera.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linux-xtensa@linux-xtensa.org \
    --cc=nios2-dev@lists.rocketboards.org \
    --cc=paulburton@kernel.org \
    --cc=rabinv@axis.com \
    --cc=ralf@linux-mips.org \
    --cc=rmk+kernel@arm.linux.org.uk \
    --cc=sparclinux@vger.kernel.org \
    --cc=steve.capper@linaro.org \
    --cc=ying.huang@intel.com \
    --cc=ysato@users.sourceforge.jp \
    /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.