From: Michal Hocko <mhocko@kernel.org>
To: John Hubbard <jhubbard@nvidia.com>
Cc: linux-api@vger.kernel.org, Khalid Aziz <khalid.aziz@oracle.com>,
Michael Ellerman <mpe@ellerman.id.au>,
Andrew Morton <akpm@linux-foundation.org>,
Russell King - ARM Linux <linux@armlinux.org.uk>,
Andrea Arcangeli <aarcange@redhat.com>,
linux-mm@kvack.org, LKML <linux-kernel@vger.kernel.org>,
linux-arch@vger.kernel.org
Subject: Re: [RFC PATCH 1/2] mm: introduce MAP_FIXED_SAFE
Date: Mon, 20 Nov 2017 10:02:39 +0100 [thread overview]
Message-ID: <20171120090239.zsm5jmgn6bvo62kg@dhcp22.suse.cz> (raw)
In-Reply-To: <be5c9478-3e03-9a1d-525c-31c904b667d8@nvidia.com>
On Fri 17-11-17 00:37:18, John Hubbard wrote:
> On 11/16/2017 02:18 AM, Michal Hocko wrote:
[...]
> > diff --git a/arch/powerpc/include/uapi/asm/mman.h b/arch/powerpc/include/uapi/asm/mman.h
> > index 03c06ba7464f..d97342ca25b1 100644
> > --- a/arch/powerpc/include/uapi/asm/mman.h
> > +++ b/arch/powerpc/include/uapi/asm/mman.h
> > @@ -28,5 +28,6 @@
> > #define MAP_NONBLOCK 0x10000 /* do not block on IO */
> > #define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
> > #define MAP_HUGETLB 0x40000 /* create a huge page mapping */
> > +#define MAP_FIXED_SAFE 0x800000 /* MAP_FIXED which doesn't unmap underlying mapping */
>
>
> Hi Michal,
>
> 1. The powerpc change, above, has one too many zeroes. It should be 0x80000,
> not 0x800000.
OK, I will fix it. It shouldn't matter much, because we only care about
non-clashing address but I agree that we should consume them from bottom
bits.
> 2. For the one-line comments, if you phrase them like this:
>
> /* Like MAP_FIXED, except that it doesn't unmap pre-existing mappings */
>
> ...I think that would be a little clearer.
I do not have any strong preference here.
[...]
> > diff --git a/mm/mmap.c b/mm/mmap.c
> > index 680506faceae..89af0b5839a5 100644
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -1342,6 +1342,10 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
> > if (!(file && path_noexec(&file->f_path)))
> > prot |= PROT_EXEC;
> >
> > + /* force arch specific MAP_FIXED handling in get_unmapped_area */
> > + if (flags & MAP_FIXED_SAFE)
> > + flags |= MAP_FIXED;
>
> Hooking in at this point is a nice way to solve the problem. :)
>
> For the naming and implementation, I see a couple of things that might improve
> it slightly:
>
> a) Change MAP_FIXED_SAFE to MAP_NO_CLOBBER (as per Kees' idea), but keep the
> new flag independent, by omitting the above two lines. Instead of forcing
> MAP_FIXED as a result of the new flag, you could simply fail to take any action
> on MAP_NO_CLOBBER *unless* MAP_FIXED is set.
>
> This is a bit easier to explain and reason about, as compared to a flag that
> auto-sets another flag. I like this approach best.
As I've exaplained in other email I do not think we can make this a
modifier.
> or
>
> b) Change MAP_FIXED_SAFE to MAP_FIXED_NO_CLOBBER (also a variation on Kees' name
> idea, but a little longer, a bit uglier, and clearer), and leave the implementation
> the same.
I do not have a _strong_ preference on the name itself. But I think that
_SAFE reflects the behavior slightly better because _NO_CLOBBER is not
very specific _when_ and _what_ we do not clobber while _SAFE is clear
on that it doesn't perform any unsafe operations.
But if the majority think that _NO_CLOBBER is better i will change it.
--
Michal Hocko
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>
next prev parent reply other threads:[~2017-11-20 9:02 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-16 10:18 Michal Hocko
2017-11-16 10:18 ` [RFC PATCH 1/2] mm: introduce MAP_FIXED_SAFE Michal Hocko
2017-11-17 0:27 ` Kees Cook
2017-11-17 19:12 ` Matthew Wilcox
2017-11-20 8:43 ` Michal Hocko
2017-11-17 7:30 ` Florian Weimer
2017-11-20 8:55 ` Michal Hocko
2017-11-20 9:10 ` Florian Weimer
2017-11-20 9:33 ` Michal Hocko
2017-11-20 9:45 ` Florian Weimer
2017-11-17 8:37 ` John Hubbard
2017-11-20 9:02 ` Michal Hocko [this message]
2017-11-16 10:19 ` [PATCH 2/2] fs, elf: drop MAP_FIXED usage from elf_map Michal Hocko
2017-11-17 0:30 ` Kees Cook
2017-11-16 12:14 ` [RFC PATCH 0/2] mm: introduce MAP_FIXED_SAFE Michal Hocko
2017-11-17 8:45 ` John Hubbard
2017-11-20 9:05 ` Michal Hocko
2017-11-22 1:48 ` John Hubbard
2017-11-22 13:12 ` Michal Hocko
2017-11-22 13:20 ` Vlastimil Babka
2017-11-24 8:54 ` Michal Hocko
2017-11-27 15:51 ` Khalid Aziz
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=20171120090239.zsm5jmgn6bvo62kg@dhcp22.suse.cz \
--to=mhocko@kernel.org \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=jhubbard@nvidia.com \
--cc=khalid.aziz@oracle.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux@armlinux.org.uk \
--cc=mpe@ellerman.id.au \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).