Skip to content

Commit

Permalink
mm/selftest: uffd: explain the write missing fault check
Browse files Browse the repository at this point in the history
It's not obvious why we had a write check for each of the missing
messages, especially when it should be a locking op.  Add a rich comment
for that, and also try to explain its good side and limitations, so that
if someone hit it again for either a bug or a different glibc impl
there'll be some clue to start with.

Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Peter Xu <[email protected]>
Reviewed-by: Mike Kravetz <[email protected]>
Reviewed-by: David Hildenbrand <[email protected]>
Cc: Andrea Arcangeli <[email protected]>
Cc: Axel Rasmussen <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Nadav Amit <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
  • Loading branch information
xzpeter authored and akpm00 committed Oct 13, 2022
1 parent f9bf6c0 commit 26c92d3
Showing 1 changed file with 21 additions and 1 deletion.
22 changes: 21 additions & 1 deletion tools/testing/selftests/vm/userfaultfd.c
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,27 @@ static void uffd_handle_page_fault(struct uffd_msg *msg,
continue_range(uffd, msg->arg.pagefault.address, page_size);
stats->minor_faults++;
} else {
/* Missing page faults */
/*
* Missing page faults.
*
* Here we force a write check for each of the missing mode
* faults. It's guaranteed because the only threads that
* will trigger uffd faults are the locking threads, and
* their first instruction to touch the missing page will
* always be pthread_mutex_lock().
*
* Note that here we relied on an NPTL glibc impl detail to
* always read the lock type at the entry of the lock op
* (pthread_mutex_t.__data.__type, offset 0x10) before
* doing any locking operations to guarantee that. It's
* actually not good to rely on this impl detail because
* logically a pthread-compatible lib can implement the
* locks without types and we can fail when linking with
* them. However since we used to find bugs with this
* strict check we still keep it around. Hopefully this
* could be a good hint when it fails again. If one day
* it'll break on some other impl of glibc we'll revisit.
*/
if (msg->arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
err("unexpected write fault");

Expand Down

0 comments on commit 26c92d3

Please sign in to comment.