mm/selftest: uffd: explain the write missing fault check

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/20221004193400.110155-4-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Axel Rasmussen <axelrasmussen@google.com>
Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
Cc: Nadav Amit <nadav.amit@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index 74babdb..297f250 100644 (file)
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -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");