From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Steve Capper <steve.capper@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com,
linux-arch@vger.kernel.org, linux-mm@kvack.org,
will.deacon@arm.com, gary.robertson@linaro.org,
christoffer.dall@linaro.org, peterz@infradead.org,
anders.roxell@linaro.org, akpm@linux-foundation.org,
dann.frazier@canonical.com, mark.rutland@arm.com,
mgorman@suse.de
Subject: Re: [PATH V2 1/6] mm: Introduce a general RCU get_user_pages_fast.
Date: Wed, 27 Aug 2014 16:01:39 +0100 [thread overview]
Message-ID: <20140827150139.GZ30401@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <1408635812-31584-2-git-send-email-steve.capper@linaro.org>
On Thu, Aug 21, 2014 at 04:43:27PM +0100, Steve Capper wrote:
> +int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> + struct page **pages)
> +{
> + struct mm_struct *mm = current->mm;
> + int nr, ret;
> +
> + start &= PAGE_MASK;
> + nr = __get_user_pages_fast(start, nr_pages, write, pages);
> + ret = nr;
> +
> + if (nr < nr_pages) {
> + /* Try to get the remaining pages with get_user_pages */
> + start += nr << PAGE_SHIFT;
> + pages += nr;
When I read this, my first reaction was... what if nr is negative? In
that case, if nr_pages is positive, we fall through into this if, and
start to wind things backwards - which isn't what we want.
It looks like that can't happen... right? __get_user_pages_fast() only
returns greater-or-equal to zero right now, but what about the future?
> +
> + down_read(&mm->mmap_sem);
> + ret = get_user_pages(current, mm, start,
> + nr_pages - nr, write, 0, pages, NULL);
> + up_read(&mm->mmap_sem);
> +
> + /* Have to be a bit careful with return values */
> + if (nr > 0) {
This kind'a makes it look like nr could be negative.
Other than that, I don't see anything obviously wrong with it.
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
WARNING: multiple messages have this Message-ID (diff)
From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Steve Capper <steve.capper@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com,
linux-arch@vger.kernel.org, linux-mm@kvack.org,
will.deacon@arm.com, gary.robertson@linaro.org,
christoffer.dall@linaro.org, peterz@infradead.org,
anders.roxell@linaro.org, akpm@linux-foundation.org,
dann.frazier@canonical.com, mark.rutland@arm.com,
mgorman@suse.de
Subject: Re: [PATH V2 1/6] mm: Introduce a general RCU get_user_pages_fast.
Date: Wed, 27 Aug 2014 16:01:39 +0100 [thread overview]
Message-ID: <20140827150139.GZ30401@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <1408635812-31584-2-git-send-email-steve.capper@linaro.org>
On Thu, Aug 21, 2014 at 04:43:27PM +0100, Steve Capper wrote:
> +int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> + struct page **pages)
> +{
> + struct mm_struct *mm = current->mm;
> + int nr, ret;
> +
> + start &= PAGE_MASK;
> + nr = __get_user_pages_fast(start, nr_pages, write, pages);
> + ret = nr;
> +
> + if (nr < nr_pages) {
> + /* Try to get the remaining pages with get_user_pages */
> + start += nr << PAGE_SHIFT;
> + pages += nr;
When I read this, my first reaction was... what if nr is negative? In
that case, if nr_pages is positive, we fall through into this if, and
start to wind things backwards - which isn't what we want.
It looks like that can't happen... right? __get_user_pages_fast() only
returns greater-or-equal to zero right now, but what about the future?
> +
> + down_read(&mm->mmap_sem);
> + ret = get_user_pages(current, mm, start,
> + nr_pages - nr, write, 0, pages, NULL);
> + up_read(&mm->mmap_sem);
> +
> + /* Have to be a bit careful with return values */
> + if (nr > 0) {
This kind'a makes it look like nr could be negative.
Other than that, I don't see anything obviously wrong with it.
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
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>
WARNING: multiple messages have this Message-ID (diff)
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATH V2 1/6] mm: Introduce a general RCU get_user_pages_fast.
Date: Wed, 27 Aug 2014 16:01:39 +0100 [thread overview]
Message-ID: <20140827150139.GZ30401@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <1408635812-31584-2-git-send-email-steve.capper@linaro.org>
On Thu, Aug 21, 2014 at 04:43:27PM +0100, Steve Capper wrote:
> +int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> + struct page **pages)
> +{
> + struct mm_struct *mm = current->mm;
> + int nr, ret;
> +
> + start &= PAGE_MASK;
> + nr = __get_user_pages_fast(start, nr_pages, write, pages);
> + ret = nr;
> +
> + if (nr < nr_pages) {
> + /* Try to get the remaining pages with get_user_pages */
> + start += nr << PAGE_SHIFT;
> + pages += nr;
When I read this, my first reaction was... what if nr is negative? In
that case, if nr_pages is positive, we fall through into this if, and
start to wind things backwards - which isn't what we want.
It looks like that can't happen... right? __get_user_pages_fast() only
returns greater-or-equal to zero right now, but what about the future?
> +
> + down_read(&mm->mmap_sem);
> + ret = get_user_pages(current, mm, start,
> + nr_pages - nr, write, 0, pages, NULL);
> + up_read(&mm->mmap_sem);
> +
> + /* Have to be a bit careful with return values */
> + if (nr > 0) {
This kind'a makes it look like nr could be negative.
Other than that, I don't see anything obviously wrong with it.
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
next prev parent reply other threads:[~2014-08-27 15:01 UTC|newest]
Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-21 15:43 [PATH V2 0/6] RCU get_user_pages_fast and __get_user_pages_fast Steve Capper
2014-08-21 15:43 ` Steve Capper
2014-08-21 15:43 ` Steve Capper
2014-08-21 15:43 ` [PATH V2 1/6] mm: Introduce a general RCU get_user_pages_fast Steve Capper
2014-08-21 15:43 ` Steve Capper
2014-08-21 15:43 ` Steve Capper
2014-08-27 8:54 ` Will Deacon
2014-08-27 8:54 ` Will Deacon
2014-08-27 8:54 ` Will Deacon
2014-08-27 12:50 ` Steve Capper
2014-08-27 12:50 ` Steve Capper
2014-08-27 12:50 ` Steve Capper
2014-08-27 13:14 ` Will Deacon
2014-08-27 13:14 ` Will Deacon
2014-08-27 13:14 ` Will Deacon
2014-08-27 14:28 ` Catalin Marinas
2014-08-27 14:28 ` Catalin Marinas
2014-08-27 14:28 ` Catalin Marinas
2014-08-27 14:42 ` Steve Capper
2014-08-27 14:42 ` Steve Capper
2014-08-27 14:42 ` Steve Capper
2014-08-27 15:01 ` Russell King - ARM Linux [this message]
2014-08-27 15:01 ` Russell King - ARM Linux
2014-08-27 15:01 ` Russell King - ARM Linux
2014-08-28 8:59 ` Steve Capper
2014-08-28 8:59 ` Steve Capper
2014-08-28 8:59 ` Steve Capper
2014-08-21 15:43 ` [PATH V2 2/6] arm: mm: Introduce special ptes for LPAE Steve Capper
2014-08-21 15:43 ` Steve Capper
2014-08-21 15:43 ` Steve Capper
2014-08-27 10:46 ` Catalin Marinas
2014-08-27 10:46 ` Catalin Marinas
2014-08-27 10:46 ` Catalin Marinas
2014-08-27 12:52 ` Steve Capper
2014-08-27 12:52 ` Steve Capper
2014-08-27 12:52 ` Steve Capper
2014-08-21 15:43 ` [PATH V2 3/6] arm: mm: Enable HAVE_RCU_TABLE_FREE logic Steve Capper
2014-08-21 15:43 ` Steve Capper
2014-08-21 15:43 ` Steve Capper
2014-08-27 11:50 ` Catalin Marinas
2014-08-27 11:50 ` Catalin Marinas
2014-08-27 11:50 ` Catalin Marinas
2014-08-27 12:59 ` Steve Capper
2014-08-27 12:59 ` Steve Capper
2014-08-27 12:59 ` Steve Capper
2014-08-21 15:43 ` [PATH V2 4/6] arm: mm: Enable RCU fast_gup Steve Capper
2014-08-21 15:43 ` Steve Capper
2014-08-21 15:43 ` Steve Capper
2014-08-27 11:51 ` Catalin Marinas
2014-08-27 11:51 ` Catalin Marinas
2014-08-27 11:51 ` Catalin Marinas
2014-08-27 13:01 ` Steve Capper
2014-08-27 13:01 ` Steve Capper
2014-08-27 13:01 ` Steve Capper
2014-08-21 15:43 ` [PATH V2 5/6] arm64: mm: Enable HAVE_RCU_TABLE_FREE logic Steve Capper
2014-08-21 15:43 ` Steve Capper
2014-08-21 15:43 ` Steve Capper
2014-08-27 10:48 ` Catalin Marinas
2014-08-27 10:48 ` Catalin Marinas
2014-08-27 10:48 ` Catalin Marinas
2014-08-27 13:08 ` Steve Capper
2014-08-27 13:08 ` Steve Capper
2014-08-27 13:08 ` Steve Capper
2014-08-21 15:43 ` [PATH V2 6/6] arm64: mm: Enable RCU fast_gup Steve Capper
2014-08-21 15:43 ` Steve Capper
2014-08-21 15:43 ` Steve Capper
2014-08-27 11:09 ` Catalin Marinas
2014-08-27 11:09 ` Catalin Marinas
2014-08-27 11:09 ` Catalin Marinas
2014-08-27 13:43 ` Steve Capper
2014-08-27 13:43 ` Steve Capper
2014-08-27 13:43 ` Steve Capper
2014-08-21 20:42 ` [PATH V2 0/6] RCU get_user_pages_fast and __get_user_pages_fast Dann Frazier
2014-08-21 20:42 ` Dann Frazier
2014-08-21 20:42 ` Dann Frazier
2014-08-22 8:11 ` Steve Capper
2014-08-22 8:11 ` Steve Capper
2014-08-22 8:11 ` Steve Capper
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=20140827150139.GZ30401@n2100.arm.linux.org.uk \
--to=linux@arm.linux.org.uk \
--cc=akpm@linux-foundation.org \
--cc=anders.roxell@linaro.org \
--cc=catalin.marinas@arm.com \
--cc=christoffer.dall@linaro.org \
--cc=dann.frazier@canonical.com \
--cc=gary.robertson@linaro.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-mm@kvack.org \
--cc=mark.rutland@arm.com \
--cc=mgorman@suse.de \
--cc=peterz@infradead.org \
--cc=steve.capper@linaro.org \
--cc=will.deacon@arm.com \
/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.