Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA balancing

From: Jan Kara <jack@suse.cz>
To: Hugh Dickins <hughd@google.com>
Cc: Lorenzo Stoakes <lstoakes@gmail.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Jan Kara <jack@suse.cz>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Rik van Riel <riel@redhat.com>, linux-mm <linux-mm@kvack.org>,
	Mel Gorman <mgorman@techsingularity.net>,
	tbsaunde@tbsaunde.org, robert@ocallahan.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA balancing
Date: Mon, 10 Oct 2016 09:47:12 +0200	[thread overview]
Message-ID: <20161010074712.GB24081@quack2.suse.cz> (raw)
In-Reply-To: <alpine.LSU.2.11.1610071101410.7822@eggly.anvils>

On Fri 07-10-16 11:16:26, Hugh Dickins wrote:
> On Fri, 7 Oct 2016, Lorenzo Stoakes wrote:
> > On Fri, Oct 07, 2016 at 08:34:15AM -0700, Linus Torvalds wrote:
> > > Would you be willing to look at doing that kind of purely syntactic,
> > > non-semantic cleanup first?
> > 
> > Sure, more than happy to do that! I'll work on a patch for this.
> > 
> > > I think that if we end up having the FOLL_FORCE semantics, we're
> > > actually better off having an explicit FOLL_FORCE flag, and *not* do
> > > some kind of implicit "under these magical circumstances we'll force
> > > it anyway". The implicit thing is what we used to do long long ago, we
> > > definitely don't want to.
> > 
> > That's a good point, it would definitely be considerably more 'magical', and
> > expanding the conditions to include uprobes etc. would only add to that.
> > 
> > I wondered about an alternative parameter/flag but it felt like it was
> > more-or-less FOLL_FORCE in a different form, at which point it may as well
> > remain FOLL_FORCE :)
> 
> Adding Jan Kara (and Dave Hansen) to the Cc list: I think they were
> pursuing get_user_pages() cleanups last year (which would remove the
> force option from most callers anyway), and I've lost track of where
> that all got to.  Lorenzo, please don't expend a lot of effort before
> checking with Jan.

Yeah, so my cleanups where mostly concerned about mmap_sem locking and
reducing number of places which cared about those. Regarding flags for
get_user_pages() / get_vaddr_frames(), I agree that using flags argument
as Linus suggests will make it easier to see what the callers actually
want. So I'm for that.

Regarding the FOLL_FORCE I've had a discussion about its use in Infiniband
drivers in 2013 with Roland Dreier. He referenced me to discussion
https://lkml.org/lkml/2012/1/26/7 and summarized that they use FOLL_FORCE
to trigger possible COW early. That avoids the situation when the process
triggers COW of the pages later after DMA buffers are set up which results
in the DMA result to not be visible where the process expects it... I'll
defer to others whether that is a sane or intended use of FOLL_FORCE flag
but I suppose that is the reason why most drivers use it when setting up
DMA buffers in memory passed from userspace.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR