tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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

This reverts commit 9857a17f206ff374aea78bccfb687f145368be2e.

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>

Linus Torvalds cd1adf1b 4b93c544

+15 -20
+1 -1
arch/s390/mm/fault.c
··· 822 822 break; 823 823 case KERNEL_FAULT: 824 824 page = phys_to_page(addr); 825 - if (unlikely(!try_get_compound_head(page, 1))) 825 + if (unlikely(!try_get_page(page))) 826 826 break; 827 827 rc = arch_make_page_accessible(page); 828 828 put_page(page);
+1 -1
fs/pipe.c
··· 191 191 */ 192 192 bool generic_pipe_buf_get(struct pipe_inode_info *pipe, struct pipe_buffer *buf) 193 193 { 194 - return try_get_compound_head(buf->page, 1); 194 + return try_get_page(buf->page); 195 195 } 196 196 EXPORT_SYMBOL(generic_pipe_buf_get); 197 197
+9 -1
include/linux/mm.h
··· 1218 1218 struct page *try_grab_compound_head(struct page *page, int refs, 1219 1219 unsigned int flags); 1220 1220 1221 - struct page *try_get_compound_head(struct page *page, int refs); 1221 + 1222 + static inline __must_check bool try_get_page(struct page *page) 1223 + { 1224 + page = compound_head(page); 1225 + if (WARN_ON_ONCE(page_ref_count(page) <= 0)) 1226 + return false; 1227 + page_ref_inc(page); 1228 + return true; 1229 + } 1222 1230 1223 1231 static inline void put_page(struct page *page) 1224 1232 {
+4 -17
mm/gup.c
··· 62 62 put_page(page); 63 63 } 64 64 65 - /** 66 - * try_get_compound_head() - return the compound head page with refcount 67 - * appropriately incremented, or NULL if that failed. 68 - * 69 - * This handles potential refcount overflow correctly. It also works correctly 70 - * for various lockless get_user_pages()-related callers, due to the use of 71 - * page_cache_add_speculative(). 72 - * 73 - * Even though the name includes "compound_head", this function is still 74 - * appropriate for callers that have a non-compound @page to get. 75 - * 76 - * @page: pointer to page to be gotten 77 - * @refs: the value to add to the page's refcount 78 - * 79 - * Return: head page (with refcount appropriately incremented) for success, or 80 - * NULL upon failure. 65 + /* 66 + * Return the compound head page with ref appropriately incremented, 67 + * or NULL if that failed. 81 68 */ 82 - struct page *try_get_compound_head(struct page *page, int refs) 69 + static inline struct page *try_get_compound_head(struct page *page, int refs) 83 70 { 84 71 struct page *head = compound_head(page); 85 72