Revert "mm/gup: remove try_get_page(), call try_get_compound_head() directly"

This reverts commit 6cd5c280661a59725d3bc5cbb8c75b4f691a6320.

That commit was completely broken, and I should have caught on to it
earlier.  But happily, the kernel test robot noticed the breakage fairly
quickly.

The breakage is because "try_get_page()" is about avoiding the page
reference count overflow case, but is otherwise the exact same as a
plain "get_page()".

In contrast, "try_get_compound_head()" is an entirely different beast,
and uses __page_cache_add_speculative() because it's not just about the
page reference count, but also about possibly racing with the underlying
page going away.

So all the commentary about how

 "try_get_page() has fallen a little behind in terms of maintenance,
  try_get_compound_head() handles speculative page references more
  thoroughly"

was just completely wrong: yes, try_get_compound_head() handles
speculative page references, but the point is that try_get_page() does
not, and must not.

So there's no lack of maintainance - there are fundamentally different
semantics.

A speculative page reference would be entirely wrong in "get_page()",
and it's entirely wrong in "try_get_page()".  It's not about
speculation, it's purely about "uhhuh, you can't get this page because
you've tried to increment the reference count too much already".

The reason the kernel test robot noticed this bug was that it hit the
VM_BUG_ON() in __page_cache_add_speculative(), which is all about
verifying that the context of any speculative page access is correct.
But since that isn't what try_get_page() is all about, the VM_BUG_ON()
tests things that are not correct to test for try_get_page().

Reported-by: kernel test robot <oliver.sang@intel.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index a834e4672f7293303d9f7cb1d767715ffca5aff7..212632d57db9c9cfd91be9acd55a5e281ece479c 100644 (file)
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -822,7 +822,7 @@ void do_secure_storage_access(struct pt_regs *regs)
                break;
        case KERNEL_FAULT:
                page = phys_to_page(addr);
-               if (unlikely(!try_get_compound_head(page, 1)))
+               if (unlikely(!try_get_page(page)))
                        break;
                rc = arch_make_page_accessible(page);
                put_page(page);

diff --git a/fs/pipe.c b/fs/pipe.c
index 1fa1f52763f0b1348ff2573dbb9040782cd164ce..6d4342bad9f15b258134fec2374955588f901de4 100644 (file)
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -191,7 +191,7 @@ EXPORT_SYMBOL(generic_pipe_buf_try_steal);
  */
 bool generic_pipe_buf_get(struct pipe_inode_info *pipe, struct pipe_buffer *buf)
 {
-       return try_get_compound_head(buf->page, 1);
+       return try_get_page(buf->page);
 }
 EXPORT_SYMBOL(generic_pipe_buf_get);
 

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 50e2c2914ac21720557c2915ada32b19efc93668..73a52aba448f940cb3b0574d61729c4436853a5e 100644 (file)
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1218,7 +1218,15 @@ bool __must_check try_grab_page(struct page *page, unsigned int flags);
 struct page *try_grab_compound_head(struct page *page, int refs,
                                    unsigned int flags);
 
-struct page *try_get_compound_head(struct page *page, int refs);
+
+static inline __must_check bool try_get_page(struct page *page)
+{
+       page = compound_head(page);
+       if (WARN_ON_ONCE(page_ref_count(page) <= 0))
+               return false;
+       page_ref_inc(page);
+       return true;
+}
 
 static inline void put_page(struct page *page)
 {

diff --git a/mm/gup.c b/mm/gup.c
index 9935a448071045b7dd5e5d9107272d667fff5ef5..886d6148d3d03afbce35cdc7352c928d00c003bc 100644 (file)
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -62,24 +62,11 @@ static void put_page_refs(struct page *page, int refs)
        put_page(page);
 }
 
-/**
- * try_get_compound_head() - return the compound head page with refcount
- * appropriately incremented, or NULL if that failed.
- *
- * This handles potential refcount overflow correctly. It also works correctly
- * for various lockless get_user_pages()-related callers, due to the use of
- * page_cache_add_speculative().
- *
- * Even though the name includes "compound_head", this function is still
- * appropriate for callers that have a non-compound @page to get.
- *
- * @page:  pointer to page to be gotten
- * @refs:  the value to add to the page's refcount
- *
- * Return: head page (with refcount appropriately incremented) for success, or
- * NULL upon failure.
+/*
+ * Return the compound head page with ref appropriately incremented,
+ * or NULL if that failed.
  */
-struct page *try_get_compound_head(struct page *page, int refs)
+static inline struct page *try_get_compound_head(struct page *page, int refs)
 {
        struct page *head = compound_head(page);