[bug report] mm/pgtable: add rcu_read_lock() and rcu_read_unlock()s

[bug report] mm/pgtable: add rcu_read_lock() and rcu_read_unlock()s

[Dan Carpenter]

[ This kind of old and mm is above my pay grade...  - dan ]

Hello Hugh Dickins,

The patch a349d72fd9ef: "mm/pgtable: add rcu_read_lock() and
rcu_read_unlock()s" from Jul 11, 2023 (linux-next), leads to the
following Smatch static checker warning:

	./include/linux/rmap.h:116 put_anon_vma()
	warn: sleeping in atomic context

./include/linux/rmap.h
    113 static inline void put_anon_vma(struct anon_vma *anon_vma)
    114 {
    115         if (atomic_dec_and_test(&anon_vma->refcount))
--> 116                 __put_anon_vma(anon_vma);
    117 }

move_pages_pte() <- disables preempt
-> folio_get_anon_vma()
   -> put_anon_vma()

The relevant bits from move_pages_pte() are here:

mm/userfaultfd.c
  1109          mmu_notifier_invalidate_range_start(&range);
  1110  retry:
  1111          dst_pte = pte_offset_map_nolock(mm, dst_pmd, dst_addr, &dst_ptl);
                          ^^^^^^^^^^^^^^^^^^^^^
  1112  
  1113          /* Retry if a huge pmd materialized from under us */
  1114          if (unlikely(!dst_pte)) {
  1115                  err = -EAGAIN;
  1116                  goto out;
  1117          }
  1118  
  1119          src_pte = pte_offset_map_nolock(mm, src_pmd, src_addr, &src_ptl);
                          ^^^^^^^^^^^^^^^^^^^^^
These call rcu_read_lock() which disables preemption.

[ snip ]

  1235                  if (!src_anon_vma) {
  1236                          /*
  1237                           * folio_referenced walks the anon_vma chain
  1238                           * without the folio lock. Serialize against it with
  1239                           * the anon_vma lock, the folio lock is not enough.
  1240                           */
  1241                          src_anon_vma = folio_get_anon_vma(src_folio);
                                               ^^^^^^^^^^^^^^^^^^
Potentially sleeps.  I started to look at if we know that folio_mapped()
will return true, but then I saw that folio_get_anon_vma() actually
checks that twice and it's some kind of parallelism thing.  I decided to
give up before I accidentally over heat my brain.

  1242                          if (!src_anon_vma) {
  1243                                  /* page was unmapped from under us */
  1244                                  err = -EAGAIN;
  1245                                  goto out;
  1246                          }
  1247                          if (!anon_vma_trylock_write(src_anon_vma)) {
  1248                                  pte_unmap(&orig_src_pte);
  1249                                  pte_unmap(&orig_dst_pte);

preempt enabled again here.

  1250                                  src_pte = dst_pte = NULL;
  1251                                  /* now we can block and wait */
  1252                                  anon_vma_lock_write(src_anon_vma);
  1253                                  goto retry;
  1254                          }
  1255                  }

regards,
dan carpenter