All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jerome Glisse <jglisse@redhat.com>
To: John Hubbard <jhubbard@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Keith Busch <keith.busch@intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-mm@kvack.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH 1/1] mm/gup_benchmark: fix MAP_HUGETLB case
Date: Tue, 22 Oct 2019 13:14:52 -0400	[thread overview]
Message-ID: <20191022171452.GA5169@redhat.com> (raw)
In-Reply-To: <20191021212435.398153-2-jhubbard@nvidia.com>

On Mon, Oct 21, 2019 at 02:24:35PM -0700, John Hubbard wrote:
> The MAP_HUGETLB ("-H" option) of gup_benchmark fails:
> 
> $ sudo ./gup_benchmark -H
> mmap: Invalid argument
> 
> This is because gup_benchmark.c is passing in a file descriptor to
> mmap(), but the fd came from opening up the /dev/zero file. This
> confuses the mmap syscall implementation, which thinks that, if the
> caller did not specify MAP_ANONYMOUS, then the file must be a huge
> page file. So it attempts to verify that the file really is a huge
> page file, as you can see here:
> 
> ksys_mmap_pgoff()
> {
>     if (!(flags & MAP_ANONYMOUS)) {
>         retval = -EINVAL;
>         if (unlikely(flags & MAP_HUGETLB && !is_file_hugepages(file)))
>             goto out_fput; /* THIS IS WHERE WE END UP */
> 
>     else if (flags & MAP_HUGETLB) {
>         ...proceed normally, /dev/zero is ok here...
> 
> ...and of course is_file_hugepages() returns "false" for the /dev/zero
> file.
> 
> The problem is that the user space program, gup_benchmark.c, really just
> wants anonymous memory here. The simplest way to get that is to pass
> MAP_ANONYMOUS whenever MAP_HUGETLB is specified, so that's what this
> patch does.

This looks wrong, MAP_HUGETLB should only be use to create vma
for hugetlbfs. If you want anonymous private vma do not set the
MAP_HUGETLB. If you want huge page inside your anonymous vma
there is nothing to do at the mmap time, this is the job of the
transparent huge page code (THP).

NAK as misleading

Cheers,
Jérôme


  reply	other threads:[~2019-10-22 17:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-21 21:24 [PATCH 0/1] mm/gup_benchmark: fix MAP_HUGETLB case John Hubbard
2019-10-21 21:24 ` [PATCH 1/1] " John Hubbard
2019-10-22 17:14   ` Jerome Glisse [this message]
2019-10-22 18:41     ` John Hubbard
2019-10-22 18:57       ` Jerome Glisse
2019-10-22 19:39         ` John Hubbard
2019-10-22 21:34           ` Andrew Morton

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=20191022171452.GA5169@redhat.com \
    --to=jglisse@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=jhubbard@nvidia.com \
    --cc=keith.busch@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.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.