Skip to content

Commit c28b1fc

Browse files
jgunthorpetorvalds
authored andcommitted
mm/gup: reorganize internal_get_user_pages_fast()
Patch series "Add a seqcount between gup_fast and copy_page_range()", v4. As discussed and suggested by Linus use a seqcount to close the small race between gup_fast and copy_page_range(). Ahmed confirms that raw_write_seqcount_begin() is the correct API to use in this case and it doesn't trigger any lockdeps. I was able to test it using two threads, one forking and the other using ibv_reg_mr() to trigger GUP fast. Modifying copy_page_range() to sleep made the window large enough to reliably hit to test the logic. This patch (of 2): The next patch in this series makes the lockless flow a little more complex, so move the entire block into a new function and remove a level of indention. Tidy a bit of cruft: - addr is always the same as start, so use start - Use the modern check_add_overflow() for computing end = start + len - nr_pinned/pages << PAGE_SHIFT needs the LHS to be unsigned long to avoid shift overflow, make the variables unsigned long to avoid coding casts in both places. nr_pinned was missing its cast - The handling of ret and nr_pinned can be streamlined a bit No functional change. Link: https://lkml.kernel.org/r/[email protected] Link: https://lkml.kernel.org/r/[email protected] Signed-off-by: Jason Gunthorpe <[email protected]> Reviewed-by: Jan Kara <[email protected]> Reviewed-by: John Hubbard <[email protected]> Reviewed-by: Peter Xu <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent d0de824 commit c28b1fc

File tree

1 file changed

+54
-45
lines changed

1 file changed

+54
-45
lines changed

mm/gup.c

Lines changed: 54 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2677,13 +2677,43 @@ static int __gup_longterm_unlocked(unsigned long start, int nr_pages,
26772677
return ret;
26782678
}
26792679

2680-
static int internal_get_user_pages_fast(unsigned long start, int nr_pages,
2680+
static unsigned long lockless_pages_from_mm(unsigned long start,
2681+
unsigned long end,
2682+
unsigned int gup_flags,
2683+
struct page **pages)
2684+
{
2685+
unsigned long flags;
2686+
int nr_pinned = 0;
2687+
2688+
if (!IS_ENABLED(CONFIG_HAVE_FAST_GUP) ||
2689+
!gup_fast_permitted(start, end))
2690+
return 0;
2691+
2692+
/*
2693+
* Disable interrupts. The nested form is used, in order to allow full,
2694+
* general purpose use of this routine.
2695+
*
2696+
* With interrupts disabled, we block page table pages from being freed
2697+
* from under us. See struct mmu_table_batch comments in
2698+
* include/asm-generic/tlb.h for more details.
2699+
*
2700+
* We do not adopt an rcu_read_lock() here as we also want to block IPIs
2701+
* that come from THPs splitting.
2702+
*/
2703+
local_irq_save(flags);
2704+
gup_pgd_range(start, end, gup_flags, pages, &nr_pinned);
2705+
local_irq_restore(flags);
2706+
return nr_pinned;
2707+
}
2708+
2709+
static int internal_get_user_pages_fast(unsigned long start,
2710+
unsigned long nr_pages,
26812711
unsigned int gup_flags,
26822712
struct page **pages)
26832713
{
2684-
unsigned long addr, len, end;
2685-
unsigned long flags;
2686-
int nr_pinned = 0, ret = 0;
2714+
unsigned long len, end;
2715+
unsigned long nr_pinned;
2716+
int ret;
26872717

26882718
if (WARN_ON_ONCE(gup_flags & ~(FOLL_WRITE | FOLL_LONGTERM |
26892719
FOLL_FORCE | FOLL_PIN | FOLL_GET |
@@ -2697,54 +2727,33 @@ static int internal_get_user_pages_fast(unsigned long start, int nr_pages,
26972727
might_lock_read(&current->mm->mmap_lock);
26982728

26992729
start = untagged_addr(start) & PAGE_MASK;
2700-
addr = start;
2701-
len = (unsigned long) nr_pages << PAGE_SHIFT;
2702-
end = start + len;
2703-
2704-
if (end <= start)
2730+
len = nr_pages << PAGE_SHIFT;
2731+
if (check_add_overflow(start, len, &end))
27052732
return 0;
27062733
if (unlikely(!access_ok((void __user *)start, len)))
27072734
return -EFAULT;
27082735

2709-
/*
2710-
* Disable interrupts. The nested form is used, in order to allow
2711-
* full, general purpose use of this routine.
2712-
*
2713-
* With interrupts disabled, we block page table pages from being
2714-
* freed from under us. See struct mmu_table_batch comments in
2715-
* include/asm-generic/tlb.h for more details.
2716-
*
2717-
* We do not adopt an rcu_read_lock(.) here as we also want to
2718-
* block IPIs that come from THPs splitting.
2719-
*/
2720-
if (IS_ENABLED(CONFIG_HAVE_FAST_GUP) && gup_fast_permitted(start, end)) {
2721-
unsigned long fast_flags = gup_flags;
2722-
2723-
local_irq_save(flags);
2724-
gup_pgd_range(addr, end, fast_flags, pages, &nr_pinned);
2725-
local_irq_restore(flags);
2726-
ret = nr_pinned;
2727-
}
2728-
2729-
if (nr_pinned < nr_pages && !(gup_flags & FOLL_FAST_ONLY)) {
2730-
/* Try to get the remaining pages with get_user_pages */
2731-
start += nr_pinned << PAGE_SHIFT;
2732-
pages += nr_pinned;
2736+
nr_pinned = lockless_pages_from_mm(start, end, gup_flags, pages);
2737+
if (nr_pinned == nr_pages || gup_flags & FOLL_FAST_ONLY)
2738+
return nr_pinned;
27332739

2734-
ret = __gup_longterm_unlocked(start, nr_pages - nr_pinned,
2735-
gup_flags, pages);
2736-
2737-
/* Have to be a bit careful with return values */
2738-
if (nr_pinned > 0) {
2739-
if (ret < 0)
2740-
ret = nr_pinned;
2741-
else
2742-
ret += nr_pinned;
2743-
}
2740+
/* Slow path: try to get the remaining pages with get_user_pages */
2741+
start += nr_pinned << PAGE_SHIFT;
2742+
pages += nr_pinned;
2743+
ret = __gup_longterm_unlocked(start, nr_pages - nr_pinned, gup_flags,
2744+
pages);
2745+
if (ret < 0) {
2746+
/*
2747+
* The caller has to unpin the pages we already pinned so
2748+
* returning -errno is not an option
2749+
*/
2750+
if (nr_pinned)
2751+
return nr_pinned;
2752+
return ret;
27442753
}
2745-
2746-
return ret;
2754+
return ret + nr_pinned;
27472755
}
2756+
27482757
/**
27492758
* get_user_pages_fast_only() - pin user pages in memory
27502759
* @start: starting user address

0 commit comments

Comments
 (0)