And there is nothing wrong about pinning an anon page that's still in the
swapcache. The following folio_test_anon() check will allow them.
The check made sense in page_mapping(), but here it's not required.
Waaaaaaaaaait a second, you were saying before:-
"Folios in the swap cache return the swap mapping" -- you might disallow
pinning anonymous pages that are in the swap cache.
I recall that there are corner cases where we can end up with an anon
page that's mapped writable but still in the swap cache ... so you'd
fallback to the GUP slow path (acceptable for these corner cases, I
guess), however especially the comment is a bit misleading then.
So are we allowing or disallowing pinning anon swap cache pages? :P
I mean slow path would allow them if they are just marked anon so I'm inclined
to allow them.
I do agree regarding folio_test_slab(), though. Should we WARN in case we
would have one?
if (WARN_ON_ONCE(folio_test_slab(folio)))
return false;
God help us if we have a slab page at this point, so agreed worth doing, it
would surely have to arise from some dreadful bug/memory corruption.
+ if (unlikely(folio_test_slab(folio) || folio_test_swapcache(folio)))
+ return false;
+
+ /* hugetlb mappings do not require dirty-tracking. */
+ if (folio_test_hugetlb(folio))
+ return true;
+
+ /*
+ * GUP-fast disables IRQs. When IRQS are disabled, RCU grace periods
+ * cannot proceed, which means no actions performed under RCU can
+ * proceed either.
+ *
+ * inodes and thus their mappings are freed under RCU, which means the
+ * mapping cannot be freed beneath us and thus we can safely dereference
+ * it.
+ */
+ lockdep_assert_irqs_disabled();
+
+ /*
+ * However, there may be operations which _alter_ the mapping, so ensure
+ * we read it once and only once.
+ */
+ mapping = READ_ONCE(folio->mapping);
+
+ /*
+ * The mapping may have been truncated, in any case we cannot determine
+ * if this mapping is safe - fall back to slow path to determine how to
+ * proceed.
+ */
+ if (!mapping)
+ return false;
+
+ /* Anonymous folios are fine, other non-file backed cases are not. */
+ mapping_flags = (unsigned long)mapping & PAGE_MAPPING_FLAGS;
+ if (mapping_flags)
+ return mapping_flags == PAGE_MAPPING_ANON;
KSM pages are also (shared) anonymous folios, and that check would fail --
which is ok (the following unsharing checks rejects long-term pinning them),
but a bit inconstent with your comment and folio_test_anon().
It would be more consistent (with your comment and also the folio_test_anon
implementation) to have here:
return mapping_flags & PAGE_MAPPING_ANON;
I explicitly excluded KSM out of fear that could be some breakage given they're
wrprotect'd + expected to CoW though? But I guess you mean they'd get picked up
by the unshare and so it doesn't matter + we wouldn't want to exclude an
PG_anon_exclusive case?
I'll make the change in any case given the unshare check!
I notice the gup_huge_pgd() doesn't do an unshare but I mean, a PGD-sized huge
page probably isn't going to be CoW'd :P