From: Linus Torvalds <torvalds@linux-foundation.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Paul Burton <paul.burton@mips.com>,
James Hogan <jhogan@kernel.org>,
Yoshinori Sato <ysato@users.sourceforge.jp>,
Rich Felker <dalias@libc.org>,
"David S. Miller" <davem@davemloft.net>,
Nicholas Piggin <npiggin@gmail.com>,
Khalid Aziz <khalid.aziz@oracle.com>,
Andrey Konovalov <andreyknvl@google.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>,
Michael Ellerman <mpe@ellerman.id.au>,
linux-mips@vger.kernel.org,
Linux-sh list <linux-sh@vger.kernel.org>,
sparclinux@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
Linux-MM <linux-mm@kvack.org>,
the arch/x86 maintainers <x86@kernel.org>,
Linux List Kernel Mailing <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 03/16] mm: simplify gup_fast_permitted
Date: Sat, 01 Jun 2019 16:14:17 +0000 [thread overview]
Message-ID: <CAHk-=whusWKhS=SYoC9f9HjVmPvR5uP51Mq=ZCtktqTBT2qiBw@mail.gmail.com> (raw)
In-Reply-To: <20190601074959.14036-4-hch@lst.de>
On Sat, Jun 1, 2019 at 12:50 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Pass in the already calculated end value instead of recomputing it, and
> leave the end > start check in the callers instead of duplicating them
> in the arch code.
Good cleanup, except it's wrong.
> - if (nr_pages <= 0)
> + if (end < start)
> return 0;
You moved the overflow test to generic code - good.
You removed the sign and zero test on nr_pages - bad.
The zero test in particular is _important_ - the GUP range operators
know and depend on the fact that they are passed a non-empty range.
The sign test it less so, but is definitely appropriate. It might be
even better to check that the "<< PAGE_SHIFT" doesn't overflow in
"long", of course, but with callers being supposed to be trusted, the
sign test at least checks for stupid underflow issues.
So at the very least that "(end < start)" needs to be "(end <start)", but honestly, I think the sign of the nr_pages should be
continued to be checked.
Linus
WARNING: multiple messages have this Message-ID (diff)
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Paul Burton <paul.burton@mips.com>,
James Hogan <jhogan@kernel.org>,
Yoshinori Sato <ysato@users.sourceforge.jp>,
Rich Felker <dalias@libc.org>,
"David S. Miller" <davem@davemloft.net>,
Nicholas Piggin <npiggin@gmail.com>,
Khalid Aziz <khalid.aziz@oracle.com>,
Andrey Konovalov <andreyknvl@google.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>,
Michael Ellerman <mpe@ellerman.id.au>,
linux-mips@vger.kernel.org,
Linux-sh list <linux-sh@vger.kernel.org>,
sparclinux@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
Linux-MM <linux-mm@kvack.org>,
"the arch/x86 maintainers" <x86@kernel.org>,
Linux List Kernel Mailing <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 03/16] mm: simplify gup_fast_permitted
Date: Sat, 1 Jun 2019 09:14:17 -0700 [thread overview]
Message-ID: <CAHk-=whusWKhS=SYoC9f9HjVmPvR5uP51Mq=ZCtktqTBT2qiBw@mail.gmail.com> (raw)
In-Reply-To: <20190601074959.14036-4-hch@lst.de>
On Sat, Jun 1, 2019 at 12:50 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Pass in the already calculated end value instead of recomputing it, and
> leave the end > start check in the callers instead of duplicating them
> in the arch code.
Good cleanup, except it's wrong.
> - if (nr_pages <= 0)
> + if (end < start)
> return 0;
You moved the overflow test to generic code - good.
You removed the sign and zero test on nr_pages - bad.
The zero test in particular is _important_ - the GUP range operators
know and depend on the fact that they are passed a non-empty range.
The sign test it less so, but is definitely appropriate. It might be
even better to check that the "<< PAGE_SHIFT" doesn't overflow in
"long", of course, but with callers being supposed to be trusted, the
sign test at least checks for stupid underflow issues.
So at the very least that "(end < start)" needs to be "(end <=
start)", but honestly, I think the sign of the nr_pages should be
continued to be checked.
Linus
WARNING: multiple messages have this Message-ID (diff)
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Paul Burton <paul.burton@mips.com>,
James Hogan <jhogan@kernel.org>,
Yoshinori Sato <ysato@users.sourceforge.jp>,
Rich Felker <dalias@libc.org>,
"David S. Miller" <davem@davemloft.net>,
Nicholas Piggin <npiggin@gmail.com>,
Khalid Aziz <khalid.aziz@oracle.com>,
Andrey Konovalov <andreyknvl@google.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>,
Michael Ellerman <mpe@ellerman.id.au>,
linux-mips@vger.kernel.org,
Linux-sh list <linux-sh@vger.kernel.org>,
sparclinux@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
Linux-MM <linux-mm@kvack.org>,
"the arch/x86 maintainers" <x86@kernel.org>,
Linux List Kernel Mailing <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 03/16] mm: simplify gup_fast_permitted
Date: Sat, 1 Jun 2019 09:14:17 -0700 [thread overview]
Message-ID: <CAHk-=whusWKhS=SYoC9f9HjVmPvR5uP51Mq=ZCtktqTBT2qiBw@mail.gmail.com> (raw)
In-Reply-To: <20190601074959.14036-4-hch@lst.de>
On Sat, Jun 1, 2019 at 12:50 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Pass in the already calculated end value instead of recomputing it, and
> leave the end > start check in the callers instead of duplicating them
> in the arch code.
Good cleanup, except it's wrong.
> - if (nr_pages <= 0)
> + if (end < start)
> return 0;
You moved the overflow test to generic code - good.
You removed the sign and zero test on nr_pages - bad.
The zero test in particular is _important_ - the GUP range operators
know and depend on the fact that they are passed a non-empty range.
The sign test it less so, but is definitely appropriate. It might be
even better to check that the "<< PAGE_SHIFT" doesn't overflow in
"long", of course, but with callers being supposed to be trusted, the
sign test at least checks for stupid underflow issues.
So at the very least that "(end < start)" needs to be "(end <=
start)", but honestly, I think the sign of the nr_pages should be
continued to be checked.
Linus
WARNING: multiple messages have this Message-ID (diff)
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Rich Felker <dalias@libc.org>,
Yoshinori Sato <ysato@users.sourceforge.jp>,
Linux-sh list <linux-sh@vger.kernel.org>,
James Hogan <jhogan@kernel.org>,
the arch/x86 maintainers <x86@kernel.org>,
Khalid Aziz <khalid.aziz@oracle.com>,
Nicholas Piggin <npiggin@gmail.com>,
linux-mips@vger.kernel.org, Linux-MM <linux-mm@kvack.org>,
Paul Burton <paul.burton@mips.com>,
Paul Mackerras <paulus@samba.org>,
Andrey Konovalov <andreyknvl@google.com>,
sparclinux@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
"David S. Miller" <davem@davemloft.net>,
Linux List Kernel Mailing <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 03/16] mm: simplify gup_fast_permitted
Date: Sat, 1 Jun 2019 09:14:17 -0700 [thread overview]
Message-ID: <CAHk-=whusWKhS=SYoC9f9HjVmPvR5uP51Mq=ZCtktqTBT2qiBw@mail.gmail.com> (raw)
In-Reply-To: <20190601074959.14036-4-hch@lst.de>
On Sat, Jun 1, 2019 at 12:50 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Pass in the already calculated end value instead of recomputing it, and
> leave the end > start check in the callers instead of duplicating them
> in the arch code.
Good cleanup, except it's wrong.
> - if (nr_pages <= 0)
> + if (end < start)
> return 0;
You moved the overflow test to generic code - good.
You removed the sign and zero test on nr_pages - bad.
The zero test in particular is _important_ - the GUP range operators
know and depend on the fact that they are passed a non-empty range.
The sign test it less so, but is definitely appropriate. It might be
even better to check that the "<< PAGE_SHIFT" doesn't overflow in
"long", of course, but with callers being supposed to be trusted, the
sign test at least checks for stupid underflow issues.
So at the very least that "(end < start)" needs to be "(end <=
start)", but honestly, I think the sign of the nr_pages should be
continued to be checked.
Linus
next prev parent reply other threads:[~2019-06-01 16:14 UTC|newest]
Thread overview: 133+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-25 13:31 RFC: switch the remaining architectures to use generic GUP Christoph Hellwig
2019-05-25 13:31 ` Christoph Hellwig
2019-05-25 13:31 ` [PATCH 1/6] MIPS: use the generic get_user_pages_fast code Christoph Hellwig
2019-05-25 13:31 ` Christoph Hellwig
2019-05-25 13:31 ` [PATCH 2/6] sh: add a missing pud_page definition Christoph Hellwig
2019-05-25 13:31 ` Christoph Hellwig
2019-05-25 13:32 ` [PATCH 3/6] sh: use the generic get_user_pages_fast code Christoph Hellwig
2019-05-25 13:32 ` Christoph Hellwig
2019-05-25 13:32 ` [PATCH 4/6] mm: add a gup_fixup_start_addr hook Christoph Hellwig
2019-05-25 13:32 ` Christoph Hellwig
2019-05-25 17:05 ` Linus Torvalds
2019-05-25 17:05 ` Linus Torvalds
2019-05-25 17:05 ` Linus Torvalds
2019-05-28 15:57 ` Khalid Aziz
2019-05-28 15:57 ` Khalid Aziz
2019-05-29 7:26 ` Christoph Hellwig
2019-05-29 7:26 ` Christoph Hellwig
2019-05-29 8:19 ` Catalin Marinas
2019-05-29 8:19 ` Catalin Marinas
2019-05-29 8:19 ` Catalin Marinas
2019-05-25 13:32 ` [PATCH 5/6] sparc64: use the generic get_user_pages_fast code Christoph Hellwig
2019-05-25 13:32 ` Christoph Hellwig
2019-05-25 16:55 ` David Miller
2019-05-25 16:55 ` David Miller
2019-05-25 13:32 ` [PATCH 6/6] mm: don't allow non-generic get_user_pages_fast implementations Christoph Hellwig
2019-05-25 13:32 ` Christoph Hellwig
2019-05-25 17:07 ` RFC: switch the remaining architectures to use generic GUP Linus Torvalds
2019-05-25 17:07 ` Linus Torvalds
2019-05-25 17:07 ` Linus Torvalds
2019-05-25 17:39 ` Christoph Hellwig
2019-05-25 17:39 ` Christoph Hellwig
2019-06-01 7:49 ` RFC: switch the remaining architectures to use generic GUP v2 Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` [PATCH 01/16] uaccess: add untagged_addr definition for other arches Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-03 15:16 ` Khalid Aziz
2019-06-03 15:16 ` Khalid Aziz
2019-06-03 15:16 ` Khalid Aziz
2019-06-04 7:27 ` Christoph Hellwig
2019-06-04 7:27 ` Christoph Hellwig
2019-06-04 7:27 ` Christoph Hellwig
2019-06-04 11:46 ` Andrey Konovalov
2019-06-04 11:46 ` Andrey Konovalov
2019-06-04 11:46 ` Andrey Konovalov
2019-06-04 11:46 ` Andrey Konovalov
2019-06-01 7:49 ` [PATCH 02/16] mm: use untagged_addr() for get_user_pages_fast addresses Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` [PATCH 03/16] mm: simplify gup_fast_permitted Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 16:14 ` Linus Torvalds [this message]
2019-06-01 16:14 ` Linus Torvalds
2019-06-01 16:14 ` Linus Torvalds
2019-06-01 16:14 ` Linus Torvalds
2019-06-03 7:41 ` Christoph Hellwig
2019-06-03 7:41 ` Christoph Hellwig
2019-06-03 7:41 ` Christoph Hellwig
2019-06-03 16:08 ` Linus Torvalds
2019-06-03 16:08 ` Linus Torvalds
2019-06-03 16:08 ` Linus Torvalds
2019-06-03 16:08 ` Linus Torvalds
2019-06-03 17:02 ` Linus Torvalds
2019-06-03 17:02 ` Linus Torvalds
2019-06-03 17:02 ` Linus Torvalds
2019-06-03 17:02 ` Linus Torvalds
2019-06-04 7:26 ` Christoph Hellwig
2019-06-04 7:26 ` Christoph Hellwig
2019-06-04 7:26 ` Christoph Hellwig
2019-06-01 7:49 ` [PATCH 04/16] mm: lift the x86_32 PAE version of gup_get_pte to common code Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` [PATCH 05/16] MIPS: use the generic get_user_pages_fast code Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` [PATCH 06/16] sh: add the missing pud_page definition Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` [PATCH 07/16] sh: use the generic get_user_pages_fast code Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` [PATCH 08/16] sparc64: add the missing pgd_page definition Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 16:28 ` Linus Torvalds
2019-06-01 16:28 ` Linus Torvalds
2019-06-01 16:28 ` Linus Torvalds
2019-06-01 16:28 ` Linus Torvalds
2019-06-03 7:44 ` Christoph Hellwig
2019-06-03 7:44 ` Christoph Hellwig
2019-06-03 7:44 ` Christoph Hellwig
2019-06-01 7:49 ` [PATCH 09/16] sparc64: define untagged_addr() Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` [PATCH 10/16] sparc64: use the generic get_user_pages_fast code Christoph Hellwig
2019-06-02 7:39 ` Hillf Danton
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-03 7:44 ` Christoph Hellwig
2019-06-03 7:44 ` Christoph Hellwig
2019-06-03 7:44 ` Christoph Hellwig
2019-06-01 7:49 ` [PATCH 11/16] mm: rename CONFIG_HAVE_GENERIC_GUP to CONFIG_HAVE_FAST_GUP Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` [PATCH 12/16] mm: consolidate the get_user_pages* implementations Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-06 6:01 ` John Hubbard
2019-06-06 6:01 ` John Hubbard
2019-06-06 6:01 ` John Hubbard
2019-06-06 6:20 ` Christoph Hellwig
2019-06-06 6:20 ` Christoph Hellwig
2019-06-06 6:20 ` Christoph Hellwig
2019-06-06 7:44 ` John Hubbard
2019-06-06 7:44 ` John Hubbard
2019-06-06 7:44 ` John Hubbard
2019-06-01 7:49 ` [PATCH 13/16] mm: validate get_user_pages_fast flags Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` [PATCH 14/16] mm: move the powerpc hugepd code to mm/gup.c Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` [PATCH 15/16] mm: switch gup_hugepte to use try_get_compound_head Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` [PATCH 16/16] mm: mark the page referenced in gup_hugepte Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 7:49 ` Christoph Hellwig
2019-06-01 23:46 ` RFC: switch the remaining architectures to use generic GUP v2 David Miller
2019-06-01 23:46 ` David Miller
2019-06-01 23:46 ` David Miller
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='CAHk-=whusWKhS=SYoC9f9HjVmPvR5uP51Mq=ZCtktqTBT2qiBw@mail.gmail.com' \
--to=torvalds@linux-foundation.org \
--cc=andreyknvl@google.com \
--cc=benh@kernel.crashing.org \
--cc=dalias@libc.org \
--cc=davem@davemloft.net \
--cc=hch@lst.de \
--cc=jhogan@kernel.org \
--cc=khalid.aziz@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-sh@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=npiggin@gmail.com \
--cc=paul.burton@mips.com \
--cc=paulus@samba.org \
--cc=sparclinux@vger.kernel.org \
--cc=x86@kernel.org \
--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.