2020-05-19 00:24:21

by John Hubbard

[permalink] [raw]
Subject: [PATCH 0/4] mm/gup, drm/i915: refactor gup_fast, convert to pin_user_pages()

This needs to go through Andrew's -mm tree, due to adding a new gup.c
routine. However, I would really love to have some testing from the
drm/i915 folks, because I haven't been able to run-time test that part
of it.

Otherwise, though, the series has passed my basic run time testing:
some LTP tests, some xfs and etx4 non-destructive xfstests, and an
assortment of other smaller ones: vm selftests, io_uring_register, a
few more. But that's only on one particular machine. Also, cross-compile
tests for half a dozen arches all pass.

Details:

In order to convert the drm/i915 driver from get_user_pages() to
pin_user_pages(), a FOLL_PIN equivalent of __get_user_pages_fast() was
required. That led to refactoring __get_user_pages_fast(), with the
following goals:

1) As above: provide a pin_user_pages*() routine for drm/i915 to call,
in place of __get_user_pages_fast(),

2) Get rid of the gup.c duplicate code for walking page tables with
interrupts disabled. This duplicate code is a minor maintenance
problem anyway.

3) Make it easy for an upcoming patch from Souptick, which aims to
convert __get_user_pages_fast() to use a gup_flags argument, instead
of a bool writeable arg. Also, if this series looks good, we can
ask Souptick to change the name as well, to whatever the consensus
is. My initial recommendation is: get_user_pages_fast_only(), to
match the new pin_user_pages_only().

John Hubbard (4):
mm/gup: move __get_user_pages_fast() down a few lines in gup.c
mm/gup: refactor and de-duplicate gup_fast() code
mm/gup: introduce pin_user_pages_fast_only()
drm/i915: convert get_user_pages() --> pin_user_pages()

drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 22 +--
include/linux/mm.h | 3 +
mm/gup.c | 150 ++++++++++++--------
3 files changed, 107 insertions(+), 68 deletions(-)


base-commit: 642b151f45dd54809ea00ecd3976a56c1ec9b53d
--
2.26.2


2020-05-19 00:25:15

by John Hubbard

[permalink] [raw]
Subject: [PATCH 2/4] mm/gup: refactor and de-duplicate gup_fast() code

There were two nearly identical sets of code for gup_fast()
style of walking the page tables with interrupts disabled.
This has lead to the usual maintenance problems that arise from
having duplicated code.

There is already a core internal routine in gup.c for gup_fast(),
so just enhance it very slightly: allow skipping the fall-back
to "slow" (regular) get_user_pages(), via the new FOLL_FAST_ONLY
flag. Then, just call internal_get_user_pages_fast() from
__get_user_pages_fast(), and adjust the API to match pre-existing
API behavior.

There is a change in behavior from this refactoring: the nested
form of interrupt disabling is used in all gup_fast() variants
now. That's because there is only one place that interrupt disabling
for page walking is done, and so the safer form is required. This
should, if anything, eliminate possible (rare) bugs, because the
non-nested form of enabling interrupts was fragile at best.

Signed-off-by: John Hubbard <[email protected]>
---
include/linux/mm.h | 1 +
mm/gup.c | 60 ++++++++++++++++++++++------------------------
2 files changed, 29 insertions(+), 32 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index a5594ac9ebe3..84b601cab699 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2782,6 +2782,7 @@ struct page *follow_page(struct vm_area_struct *vma, unsigned long address,
#define FOLL_LONGTERM 0x10000 /* mapping lifetime is indefinite: see below */
#define FOLL_SPLIT_PMD 0x20000 /* split huge pmd before returning */
#define FOLL_PIN 0x40000 /* pages must be released via unpin_user_page */
+#define FOLL_FAST_ONLY 0x80000 /* gup_fast: prevent fall-back to slow gup */

/*
* FOLL_PIN and FOLL_LONGTERM may be used in various combinations with each
diff --git a/mm/gup.c b/mm/gup.c
index 4502846d57f9..bb3e2c4288c3 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2694,6 +2694,7 @@ static int internal_get_user_pages_fast(unsigned long start, int nr_pages,
struct page **pages)
{
unsigned long addr, len, end;
+ unsigned long flags;
int nr_pinned = 0, ret = 0;

if (WARN_ON_ONCE(gup_flags & ~(FOLL_WRITE | FOLL_LONGTERM |
@@ -2710,15 +2711,26 @@ static int internal_get_user_pages_fast(unsigned long start, int nr_pages,
if (unlikely(!access_ok((void __user *)start, len)))
return -EFAULT;

+ /*
+ * Disable interrupts. The nested form is used, in order to allow full,
+ * general purpose use of this routine.
+ *
+ * With interrupts disabled, we block page table pages from being
+ * freed from under us. See struct mmu_table_batch comments in
+ * include/asm-generic/tlb.h for more details.
+ *
+ * We do not adopt an rcu_read_lock(.) here as we also want to
+ * block IPIs that come from THPs splitting.
+ */
if (IS_ENABLED(CONFIG_HAVE_FAST_GUP) &&
gup_fast_permitted(start, end)) {
- local_irq_disable();
+ local_irq_save(flags);
gup_pgd_range(addr, end, gup_flags, pages, &nr_pinned);
- local_irq_enable();
+ local_irq_restore(flags);
ret = nr_pinned;
}

- if (nr_pinned < nr_pages) {
+ if (nr_pinned < nr_pages && !(gup_flags & FOLL_FAST_ONLY)) {
/* Try to get the remaining pages with get_user_pages */
start += nr_pinned << PAGE_SHIFT;
pages += nr_pinned;
@@ -2750,45 +2762,29 @@ static int internal_get_user_pages_fast(unsigned long start, int nr_pages,
int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
struct page **pages)
{
- unsigned long len, end;
- unsigned long flags;
- int nr_pinned = 0;
+ int nr_pinned;
/*
* Internally (within mm/gup.c), gup fast variants must set FOLL_GET,
* because gup fast is always a "pin with a +1 page refcount" request.
+ *
+ * FOLL_FAST_ONLY is required in order to match the API description of
+ * this routine: no fall back to regular ("slow") GUP.
*/
- unsigned int gup_flags = FOLL_GET;
+ unsigned int gup_flags = FOLL_GET | FOLL_FAST_ONLY;

if (write)
gup_flags |= FOLL_WRITE;

- start = untagged_addr(start) & PAGE_MASK;
- len = (unsigned long) nr_pages << PAGE_SHIFT;
- end = start + len;
-
- if (end <= start)
- return 0;
- if (unlikely(!access_ok((void __user *)start, len)))
- return 0;
-
+ nr_pinned = internal_get_user_pages_fast(start, nr_pages, gup_flags,
+ pages);
/*
- * Disable interrupts. We use the nested form as we can already have
- * interrupts disabled by get_futex_key.
- *
- * With interrupts disabled, we block page table pages from being
- * freed from under us. See struct mmu_table_batch comments in
- * include/asm-generic/tlb.h for more details.
- *
- * We do not adopt an rcu_read_lock(.) here as we also want to
- * block IPIs that come from THPs splitting.
+ * As specified in the API description above, this routine is not
+ * allowed to return negative values. However, the common core
+ * routine internal_get_user_pages_fast() *can* return -errno.
+ * Therefore, correct for that here:
*/
-
- if (IS_ENABLED(CONFIG_HAVE_FAST_GUP) &&
- gup_fast_permitted(start, end)) {
- local_irq_save(flags);
- gup_pgd_range(start, end, gup_flags, pages, &nr_pinned);
- local_irq_restore(flags);
- }
+ if (nr_pinned < 0)
+ nr_pinned = 0;

return nr_pinned;
}
--
2.26.2

2020-05-19 00:25:18

by John Hubbard

[permalink] [raw]
Subject: [PATCH 3/4] mm/gup: introduce pin_user_pages_fast_only()

This is the FOLL_PIN equivalent of __get_user_pages_fast(),
except with a more descriptive name, and gup_flags instead of
a boolean "write" in the argument list.

Signed-off-by: John Hubbard <[email protected]>
---
include/linux/mm.h | 2 ++
mm/gup.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 84b601cab699..98be7289d7e9 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1820,6 +1820,8 @@ extern int mprotect_fixup(struct vm_area_struct *vma,
*/
int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
struct page **pages);
+int pin_user_pages_fast_only(unsigned long start, int nr_pages,
+ unsigned int gup_flags, struct page **pages);
/*
* per-process(per-mm_struct) statistics.
*/
diff --git a/mm/gup.c b/mm/gup.c
index bb3e2c4288c3..4413f0f94b68 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2858,6 +2858,42 @@ int pin_user_pages_fast(unsigned long start, int nr_pages,
}
EXPORT_SYMBOL_GPL(pin_user_pages_fast);

+/*
+ * This is the FOLL_PIN equivalent of __get_user_pages_fast(). Behavior is the
+ * same, except that this one sets FOLL_PIN instead of FOLL_GET.
+ *
+ * The API rules are the same, too: no negative values may be returned.
+ */
+int pin_user_pages_fast_only(unsigned long start, int nr_pages,
+ unsigned int gup_flags, struct page **pages)
+{
+ int nr_pinned;
+
+ /*
+ * FOLL_GET and FOLL_PIN are mutually exclusive. Note that the API
+ * rules require returning 0, rather than -errno:
+ */
+ if (WARN_ON_ONCE(gup_flags & FOLL_GET))
+ return 0;
+ /*
+ * FOLL_FAST_ONLY is required in order to match the API description of
+ * this routine: no fall back to regular ("slow") GUP.
+ */
+ gup_flags |= (FOLL_PIN | FOLL_FAST_ONLY);
+ nr_pinned = internal_get_user_pages_fast(start, nr_pages, gup_flags,
+ pages);
+ /*
+ * This routine is not allowed to return negative values. However,
+ * internal_get_user_pages_fast() *can* return -errno. Therefore,
+ * correct for that here:
+ */
+ if (nr_pinned < 0)
+ nr_pinned = 0;
+
+ return nr_pinned;
+}
+EXPORT_SYMBOL_GPL(pin_user_pages_fast_only);
+
/**
* pin_user_pages_remote() - pin pages of a remote process (task != current)
*
--
2.26.2

2020-05-21 18:59:52

by Chris Wilson

[permalink] [raw]
Subject: Re: [PATCH 0/4] mm/gup, drm/i915: refactor gup_fast, convert to pin_user_pages()

Quoting John Hubbard (2020-05-19 01:21:20)
> This needs to go through Andrew's -mm tree, due to adding a new gup.c
> routine. However, I would really love to have some testing from the
> drm/i915 folks, because I haven't been able to run-time test that part
> of it.

CI hit

<4> [185.667750] WARNING: CPU: 0 PID: 1387 at mm/gup.c:2699 internal_get_user_pages_fast+0x63a/0xac0
<4> [185.667752] Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915 mei_hdcp x86_pkg_temp_thermal coretemp snd_hda_intel snd_intel_dspcfg crct10dif_pclmul snd_hda_codec crc32_pclmul snd_hwdep snd_hda_core ghash_clmulni_intel cdc_ether usbnet mii snd_pcm e1000e mei_me ptp pps_core mei intel_lpss_pci prime_numbers
<4> [185.667774] CPU: 0 PID: 1387 Comm: gem_userptr_bli Tainted: G U 5.7.0-rc5-CI-Patchwork_17704+ #1
<4> [185.667777] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP, BIOS ICLSFWR1.R00.3234.A01.1906141750 06/14/2019
<4> [185.667782] RIP: 0010:internal_get_user_pages_fast+0x63a/0xac0
<4> [185.667785] Code: 24 40 08 48 39 5c 24 38 49 89 df 0f 85 74 fc ff ff 48 83 44 24 50 08 48 39 5c 24 58 49 89 dc 0f 85 e0 fb ff ff e9 14 fe ff ff <0f> 0b b8 ea ff ff ff e9 36 fb ff ff 4c 89 e8 48 21 e8 48 39 e8 0f
<4> [185.667789] RSP: 0018:ffffc90001133c38 EFLAGS: 00010206
<4> [185.667792] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff8884999ee800
<4> [185.667795] RDX: 00000000000c0001 RSI: 0000000000000100 RDI: 00007f419e774000
<4> [185.667798] RBP: ffff888453dbf040 R08: 0000000000000000 R09: 0000000000000001
<4> [185.667800] R10: 0000000000000000 R11: 0000000000000000 R12: ffff888453dbf380
<4> [185.667803] R13: ffff8884999ee800 R14: ffff888453dbf3e8 R15: 0000000000000040
<4> [185.667806] FS: 00007f419e875e40(0000) GS:ffff88849fe00000(0000) knlGS:0000000000000000
<4> [185.667808] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4> [185.667811] CR2: 00007f419e873000 CR3: 0000000458bd2004 CR4: 0000000000760ef0
<4> [185.667814] PKRU: 55555554
<4> [185.667816] Call Trace:
<4> [185.667912] ? i915_gem_userptr_get_pages+0x1c6/0x290 [i915]
<4> [185.667918] ? mark_held_locks+0x49/0x70
<4> [185.667998] ? i915_gem_userptr_get_pages+0x1c6/0x290 [i915]
<4> [185.668073] ? i915_gem_userptr_get_pages+0x1c6/0x290 [i915]

and then panicked, across a range of systems.
-Chris

2020-05-21 19:12:45

by John Hubbard

[permalink] [raw]
Subject: Re: [PATCH 0/4] mm/gup, drm/i915: refactor gup_fast, convert to pin_user_pages()

On 2020-05-21 11:57, Chris Wilson wrote:
> Quoting John Hubbard (2020-05-19 01:21:20)
>> This needs to go through Andrew's -mm tree, due to adding a new gup.c
>> routine. However, I would really love to have some testing from the
>> drm/i915 folks, because I haven't been able to run-time test that part
>> of it.
>
> CI hit
>
> <4> [185.667750] WARNING: CPU: 0 PID: 1387 at mm/gup.c:2699 internal_get_user_pages_fast+0x63a/0xac0
> <4> [185.667752] Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915 mei_hdcp x86_pkg_temp_thermal coretemp snd_hda_intel snd_intel_dspcfg crct10dif_pclmul snd_hda_codec crc32_pclmul snd_hwdep snd_hda_core ghash_clmulni_intel cdc_ether usbnet mii snd_pcm e1000e mei_me ptp pps_core mei intel_lpss_pci prime_numbers
> <4> [185.667774] CPU: 0 PID: 1387 Comm: gem_userptr_bli Tainted: G U 5.7.0-rc5-CI-Patchwork_17704+ #1
> <4> [185.667777] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP, BIOS ICLSFWR1.R00.3234.A01.1906141750 06/14/2019
> <4> [185.667782] RIP: 0010:internal_get_user_pages_fast+0x63a/0xac0
> <4> [185.667785] Code: 24 40 08 48 39 5c 24 38 49 89 df 0f 85 74 fc ff ff 48 83 44 24 50 08 48 39 5c 24 58 49 89 dc 0f 85 e0 fb ff ff e9 14 fe ff ff <0f> 0b b8 ea ff ff ff e9 36 fb ff ff 4c 89 e8 48 21 e8 48 39 e8 0f
> <4> [185.667789] RSP: 0018:ffffc90001133c38 EFLAGS: 00010206
> <4> [185.667792] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff8884999ee800
> <4> [185.667795] RDX: 00000000000c0001 RSI: 0000000000000100 RDI: 00007f419e774000
> <4> [185.667798] RBP: ffff888453dbf040 R08: 0000000000000000 R09: 0000000000000001
> <4> [185.667800] R10: 0000000000000000 R11: 0000000000000000 R12: ffff888453dbf380
> <4> [185.667803] R13: ffff8884999ee800 R14: ffff888453dbf3e8 R15: 0000000000000040
> <4> [185.667806] FS: 00007f419e875e40(0000) GS:ffff88849fe00000(0000) knlGS:0000000000000000
> <4> [185.667808] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> <4> [185.667811] CR2: 00007f419e873000 CR3: 0000000458bd2004 CR4: 0000000000760ef0
> <4> [185.667814] PKRU: 55555554
> <4> [185.667816] Call Trace:
> <4> [185.667912] ? i915_gem_userptr_get_pages+0x1c6/0x290 [i915]
> <4> [185.667918] ? mark_held_locks+0x49/0x70
> <4> [185.667998] ? i915_gem_userptr_get_pages+0x1c6/0x290 [i915]
> <4> [185.668073] ? i915_gem_userptr_get_pages+0x1c6/0x290 [i915]
>
> and then panicked, across a range of systems.
> -Chris
>

Thanks for this report! I'm looking into it now.

thanks,
--
John Hubbard
NVIDIA

2020-05-21 20:42:40

by John Hubbard

[permalink] [raw]
Subject: Solved: [PATCH 0/4] mm/gup, drm/i915: refactor gup_fast, convert to pin_user_pages()

On 2020-05-21 12:11, John Hubbard wrote:
> On 2020-05-21 11:57, Chris Wilson wrote:
>> Quoting John Hubbard (2020-05-19 01:21:20)
>>> This needs to go through Andrew's -mm tree, due to adding a new gup.c
>>> routine. However, I would really love to have some testing from the
>>> drm/i915 folks, because I haven't been able to run-time test that part
>>> of it.
>>
>> CI hit
>>
>> <4> [185.667750] WARNING: CPU: 0 PID: 1387 at mm/gup.c:2699
>> internal_get_user_pages_fast+0x63a/0xac0


OK, what happened here is that it's WARN()'ing due to passing in the new
FOLL_FAST_ONLY flag, which was not added to the whitelist.

So the fix is easy, and should be applied to the refactoring patch. I'll
send out a v2 of the series, which will effectively have this applied:


diff --git a/mm/gup.c b/mm/gup.c
index 6cbe98c93466..4f0ca3f849d1 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2696,7 +2696,8 @@ static int internal_get_user_pages_fast(unsigned long start,
int nr_pages,
int nr_pinned = 0, ret = 0;

if (WARN_ON_ONCE(gup_flags & ~(FOLL_WRITE | FOLL_LONGTERM |
- FOLL_FORCE | FOLL_PIN | FOLL_GET)))
+ FOLL_FORCE | FOLL_PIN | FOLL_GET |
+ FOLL_FAST_ONLY)))
return -EINVAL;

start = untagged_addr(start) & PAGE_MASK;


>> <4> [185.667752] Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_realtek
>> snd_hda_codec_generic i915 mei_hdcp x86_pkg_temp_thermal coretemp snd_hda_intel
>> snd_intel_dspcfg crct10dif_pclmul snd_hda_codec crc32_pclmul snd_hwdep snd_hda_core
>> ghash_clmulni_intel cdc_ether usbnet mii snd_pcm e1000e mei_me ptp pps_core mei
>> intel_lpss_pci prime_numbers
>> <4> [185.667774] CPU: 0 PID: 1387 Comm: gem_userptr_bli Tainted: G     U
>> 5.7.0-rc5-CI-Patchwork_17704+ #1
>> <4> [185.667777] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake
>> U DDR4 SODIMM PD RVP, BIOS ICLSFWR1.R00.3234.A01.1906141750 06/14/2019
>> <4> [185.667782] RIP: 0010:internal_get_user_pages_fast+0x63a/0xac0
>> <4> [185.667785] Code: 24 40 08 48 39 5c 24 38 49 89 df 0f 85 74 fc ff ff 48 83 44
>> 24 50 08 48 39 5c 24 58 49 89 dc 0f 85 e0 fb ff ff e9 14 fe ff ff <0f> 0b b8 ea ff
>> ff ff e9 36 fb ff ff 4c 89 e8 48 21 e8 48 39 e8 0f
>> <4> [185.667789] RSP: 0018:ffffc90001133c38 EFLAGS: 00010206
>> <4> [185.667792] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff8884999ee800
>> <4> [185.667795] RDX: 00000000000c0001 RSI: 0000000000000100 RDI: 00007f419e774000
>> <4> [185.667798] RBP: ffff888453dbf040 R08: 0000000000000000 R09: 0000000000000001
>> <4> [185.667800] R10: 0000000000000000 R11: 0000000000000000 R12: ffff888453dbf380
>> <4> [185.667803] R13: ffff8884999ee800 R14: ffff888453dbf3e8 R15: 0000000000000040
>> <4> [185.667806] FS:  00007f419e875e40(0000) GS:ffff88849fe00000(0000)
>> knlGS:0000000000000000
>> <4> [185.667808] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> <4> [185.667811] CR2: 00007f419e873000 CR3: 0000000458bd2004 CR4: 0000000000760ef0
>> <4> [185.667814] PKRU: 55555554
>> <4> [185.667816] Call Trace:
>> <4> [185.667912]  ? i915_gem_userptr_get_pages+0x1c6/0x290 [i915]
>> <4> [185.667918]  ? mark_held_locks+0x49/0x70
>> <4> [185.667998]  ? i915_gem_userptr_get_pages+0x1c6/0x290 [i915]
>> <4> [185.668073]  ? i915_gem_userptr_get_pages+0x1c6/0x290 [i915]
>>
>> and then panicked, across a range of systems.
>> -Chris
>>

btw, the panic seems to indicate an additional, pre-existing problem:
i915_gem_userptr_get_pages(), in this case at least, is not able to
recover from a get_user_pages/pin_user_pages failure.


thanks,
--
John Hubbard
NVIDIA

2020-05-22 11:42:36

by Souptick Joarder

[permalink] [raw]
Subject: Re: [PATCH 0/4] mm/gup, drm/i915: refactor gup_fast, convert to pin_user_pages()

Hi John,


On Tue, May 19, 2020 at 5:51 AM John Hubbard <[email protected]> wrote:
>
> This needs to go through Andrew's -mm tree, due to adding a new gup.c
> routine. However, I would really love to have some testing from the
> drm/i915 folks, because I haven't been able to run-time test that part
> of it.
>
> Otherwise, though, the series has passed my basic run time testing:
> some LTP tests, some xfs and etx4 non-destructive xfstests, and an
> assortment of other smaller ones: vm selftests, io_uring_register, a
> few more. But that's only on one particular machine. Also, cross-compile
> tests for half a dozen arches all pass.
>
> Details:
>
> In order to convert the drm/i915 driver from get_user_pages() to
> pin_user_pages(), a FOLL_PIN equivalent of __get_user_pages_fast() was
> required. That led to refactoring __get_user_pages_fast(), with the
> following goals:
>
> 1) As above: provide a pin_user_pages*() routine for drm/i915 to call,
> in place of __get_user_pages_fast(),
>
> 2) Get rid of the gup.c duplicate code for walking page tables with
> interrupts disabled. This duplicate code is a minor maintenance
> problem anyway.
>
> 3) Make it easy for an upcoming patch from Souptick, which aims to
> convert __get_user_pages_fast() to use a gup_flags argument, instead
> of a bool writeable arg. Also, if this series looks good, we can
> ask Souptick to change the name as well, to whatever the consensus
> is. My initial recommendation is: get_user_pages_fast_only(), to
> match the new pin_user_pages_only().

Shall I hold my changes till 5.8-rc1 , when this series will appear upstream ?
>
> John Hubbard (4):
> mm/gup: move __get_user_pages_fast() down a few lines in gup.c
> mm/gup: refactor and de-duplicate gup_fast() code
> mm/gup: introduce pin_user_pages_fast_only()
> drm/i915: convert get_user_pages() --> pin_user_pages()
>
> drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 22 +--
> include/linux/mm.h | 3 +
> mm/gup.c | 150 ++++++++++++--------
> 3 files changed, 107 insertions(+), 68 deletions(-)
>
>
> base-commit: 642b151f45dd54809ea00ecd3976a56c1ec9b53d
> --
> 2.26.2
>

2020-05-22 22:24:51

by John Hubbard

[permalink] [raw]
Subject: Re: [PATCH 0/4] mm/gup, drm/i915: refactor gup_fast, convert to pin_user_pages()

On 2020-05-22 04:40, Souptick Joarder wrote:
...
>> 3) Make it easy for an upcoming patch from Souptick, which aims to
>> convert __get_user_pages_fast() to use a gup_flags argument, instead
>> of a bool writeable arg. Also, if this series looks good, we can
>> ask Souptick to change the name as well, to whatever the consensus
>> is. My initial recommendation is: get_user_pages_fast_only(), to
>> match the new pin_user_pages_only().
>
> Shall I hold my changes till 5.8-rc1 , when this series will appear upstream ?

I don't really see any problem with your posting something that is based on
the latest linux-next (which has my changes now). Should be fine. And in
fact it would be nice to get that done in this round, so that the pin* and
get* APIs look the same.


thanks,
--
John Hubbard
NVIDIA