From: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
To: John Hubbard <jhubbard@nvidia.com>, Jann Horn <jannh@google.com>
Cc: mtk.manpages@gmail.com, linux-man <linux-man@vger.kernel.org>,
Michal Hocko <mhocko@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Linux-MM <linux-mm@kvack.org>,
lkml <linux-kernel@vger.kernel.org>,
Linux API <linux-api@vger.kernel.org>
Subject: Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved
Date: Fri, 13 Apr 2018 08:43:27 +0200 [thread overview]
Message-ID: <9c714917-fc29-4d12-b5e8-cff28761a2c1@gmail.com> (raw)
In-Reply-To: <cfbbbe06-5e63-e43c-fb28-c5afef9e1e1d@nvidia.com>
On 04/12/2018 09:24 PM, John Hubbard wrote:
> On 04/12/2018 12:18 PM, Jann Horn wrote:
>> On Thu, Apr 12, 2018 at 8:59 PM, John Hubbard <jhubbard@nvidia.com> wrote:
>>> On 04/12/2018 11:49 AM, Jann Horn wrote:
>>>> On Thu, Apr 12, 2018 at 8:37 PM, Michael Kerrisk (man-pages)
>>>> <mtk.manpages@gmail.com> wrote:
>>>>> Hi John,
>>>>>
>>>>> On 12 April 2018 at 20:33, John Hubbard <jhubbard@nvidia.com> wrote:
>>>>>> On 04/12/2018 08:39 AM, Jann Horn wrote:
>>>>>>> Clarify that MAP_FIXED is appropriate if the specified address range has
>>>>>>> been reserved using an existing mapping, but shouldn't be used otherwise.
>>>>>>>
>>>>>>> Signed-off-by: Jann Horn <jannh@google.com>
>>>>>>> ---
>>>>>>> man2/mmap.2 | 19 +++++++++++--------
>>>>>>> 1 file changed, 11 insertions(+), 8 deletions(-)
>>>>>>>
>>>>>>> diff --git a/man2/mmap.2 b/man2/mmap.2
>>>> [...]
>>>>>>> .IP
>>>>>>> For example, suppose that thread A looks through
>>>>>>> @@ -284,13 +285,15 @@ and the PAM libraries
>>>>>>> .UR http://www.linux-pam.org
>>>>>>> .UE .
>>>>>>> .IP
>>>>>>> -Newer kernels
>>>>>>> -(Linux 4.17 and later) have a
>>>>>>> +For cases in which the specified memory region has not been reserved using an
>>>>>>> +existing mapping, newer kernels (Linux 4.17 and later) provide an option
>>>>>>> .B MAP_FIXED_NOREPLACE
>>>>>>> -option that avoids the corruption problem; if available,
>>>>>>> -.B MAP_FIXED_NOREPLACE
>>>>>>> -should be preferred over
>>>>>>> -.BR MAP_FIXED .
>>>>>>> +that should be used instead; older kernels require the caller to use
>>>>>>> +.I addr
>>>>>>> +as a hint (without
>>>>>>> +.BR MAP_FIXED )
>>>>>>
>>>>>> Here, I got lost: the sentence suddenly jumps into explaining non-MAP_FIXED
>>>>>> behavior, in the MAP_FIXED section. Maybe if you break up the sentence, and
>>>>>> possibly omit non-MAP_FIXED discussion, it will help.
>>>>>
>>>>> Hmmm -- true. That piece could be a little clearer.
>>>>
>>>> How about something like this?
>>>>
>>>> For cases in which MAP_FIXED can not be used because
>>>> the specified memory
>>>> region has not been reserved using an existing mapping,
>>>> newer kernels
>>>> (Linux 4.17 and later) provide an option
>>>> MAP_FIXED_NOREPLACE that
>>>> should be used instead. Older kernels require the
>>>> caller to use addr as a hint and take appropriate action if
>>>> the kernel places the new mapping at a different address.
>>>>
>>>> John, Michael, what do you think?
>>>
>>>
>>> I'm still having difficulty with it, because this is in the MAP_FIXED section,
>>> but I think you're documenting the behavior that you get if you do *not*
>>> specify MAP_FIXED, right? Also, the hint behavior is true of both older and
>>> new kernels...
>>
>> The manpage patch you and mhocko wrote mentioned MAP_FIXED_NOREPLACE
>> in the MAP_FIXED section - I was trying to avoid undoing a change you
>> had just explicitly made.
>
> heh. So I've succeeding in getting my own wording removed, then? Progress! :)
>
>>
>>> So, if that's your intent (you want to sort of document by contrast to what
>>> would happen if this option were not used), then how about something like this:
>>>
>>>
>>> Without the MAP_FIXED option, the kernel would treat addr as a hint, rather
>>> than a requirement, and the caller would need to take appropriate action
>>> if the kernel placed the mapping at a different address. (For example,
>>> munmap and try again.)
>>
>> I'd be fine with removing the paragraph. As you rightly pointed out,
>> it doesn't really describe MAP_FIXED.
>>
>
> OK, that's probably the simplest fix.
So, you mean remove this entire paragraph:
For cases in which the specified memory region has not been
reserved using an existing mapping, newer kernels (Linux
4.17 and later) provide an option MAP_FIXED_NOREPLACE that
should be used instead; older kernels require the caller to
use addr as a hint (without MAP_FIXED) and take appropriate
action if the kernel places the new mapping at a different
address.
It seems like some version of the first half of the paragraph is worth
keeping, though, so as to point the reader in the direction of a remedy.
How about replacing that text with the following:
Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
in a multithreaded program to avoid the hazard described
above.
?
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
next prev parent reply other threads:[~2018-04-13 6:43 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-12 15:39 [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved Jann Horn
2018-04-12 15:39 ` Jann Horn
2018-04-12 15:39 ` Jann Horn
2018-04-12 18:32 ` Michael Kerrisk (man-pages)
2018-04-12 18:33 ` John Hubbard
2018-04-12 18:33 ` John Hubbard
2018-04-12 18:37 ` Michael Kerrisk (man-pages)
2018-04-12 18:49 ` Jann Horn
2018-04-12 18:59 ` John Hubbard
2018-04-12 19:18 ` Jann Horn
2018-04-12 19:24 ` John Hubbard
2018-04-13 6:43 ` Michael Kerrisk (man-pages) [this message]
2018-04-13 6:49 ` Michal Hocko
2018-04-13 15:04 ` Jann Horn
2018-04-13 16:04 ` Michal Hocko
2018-04-13 16:05 ` Jann Horn
2018-04-13 16:17 ` Jann Horn
2018-04-16 10:07 ` Michal Hocko
2018-04-16 13:55 ` Jann Horn
2018-04-16 19:18 ` Michal Hocko
2018-04-16 19:30 ` Jann Horn
2018-04-16 19:57 ` Michal Hocko
2018-04-16 20:17 ` Jann Horn
2018-04-16 21:11 ` Michal Hocko
2018-04-16 21:12 ` Jann Horn
2018-04-17 6:23 ` Michal Hocko
2018-05-02 13:06 ` Michael Kerrisk (man-pages)
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=9c714917-fc29-4d12-b5e8-cff28761a2c1@gmail.com \
--to=mtk.manpages@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=jannh@google.com \
--cc=jhubbard@nvidia.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-man@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
/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.