[v4,0/2] Add a seqcount between gup_fast and copy_page_range()

Message

Jason Gunthorpe Nov. 10, 2020, 11:44 p.m. UTC

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.

v4:
 - Use read_seqcount_retry() not read_seqcount_t_retry
v3: https://lore.kernel.org/r/[e-mail]
 - Revise comment for write_protect_seq
 - Revise comment in copy_page_range
 - Use raw_write_seqcount_begin() not raw_write_seqcount_t_begin()
v2: https://lore.kernel.org/r/[e-mail]
 - Use start not addr in lockless_pages_from_mm
 - Replace unsigned long casts with using the proper variable type
 - Update comments
 - Use raw_write_seqcount_t_begin() instead of open coding
 - Update commit messages
v1: https://lore.kernel.org/r/[e-mail]

To: [e-mail]
To: Peter Xu <[e-mail]>
To: Linus Torvalds <[e-mail]>
Cc: Peter Xu <[e-mail]>
Cc: John Hubbard <[e-mail]>
Cc: Linux-MM <[e-mail]>
Cc: Linux Kernel Mailing List <[e-mail]>
Cc: Andrew Morton <[e-mail]>
Cc: Jan Kara <[e-mail]>
Cc: Michal Hocko <[e-mail]>
Cc: Kirill Tkhai <[e-mail]>
Cc: Kirill Shutemov <[e-mail]>
Cc: Hugh Dickins <[e-mail]>
Cc: Christoph Hellwig <[e-mail]>
Cc: Andrea Arcangeli <[e-mail]>
Cc: Oleg Nesterov <[e-mail]>
Cc: Jann Horn <[e-mail]>
Cc: "Ahmed S. Darwish" <[e-mail]>

Jason Gunthorpe (2):
  mm: reorganize internal_get_user_pages_fast()
  mm: prevent gup_fast from racing with COW during fork

 arch/x86/kernel/tboot.c    |   1 +
 drivers/firmware/efi/efi.c |   1 +
 include/linux/mm_types.h   |   8 +++
 kernel/fork.c              |   1 +
 mm/gup.c                   | 117 +++++++++++++++++++++++--------------
 mm/init-mm.c               |   1 +
 mm/memory.c                |  13 ++++-
 7 files changed, 96 insertions(+), 46 deletions(-)

Comments

Linus Torvalds Nov. 11, 2020, 6:27 p.m. UTC | #1

On Tue, Nov 10, 2020 at 3:44 PM Jason Gunthorpe <[e-mail]> wrote:
>
> 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.

Looks all good to me.

             Linus