| Message ID | 20210808235018.1924918-4-jhubbard@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | A few gup refactorings and documentation updates | expand |
On Sun, Aug 08, 2021 at 04:50:18PM -0700, John Hubbard wrote: > +/* > + * This has the same functionality as try_get_compound_head(), just with a > + * slightly different API. > + */ > static inline __must_check bool try_get_page(struct page *page) > { > + return try_get_compound_head(page, 1) != NULL; What about removing try_get_page entirely instead?
On 8/8/21 11:29 PM, Christoph Hellwig wrote: > On Sun, Aug 08, 2021 at 04:50:18PM -0700, John Hubbard wrote: >> +/* >> + * This has the same functionality as try_get_compound_head(), just with a >> + * slightly different API. >> + */ >> static inline __must_check bool try_get_page(struct page *page) >> { >> + return try_get_compound_head(page, 1) != NULL; > > What about removing try_get_page entirely instead? > This thought admittedly crossed my mind, but I was worried maybe I was getting carried away with cleanups. But now that at least one other person thinks that's reasonable, I'll be glad to cleanup the callers too. thanks,
diff --git a/include/linux/mm.h b/include/linux/mm.h index ce8fc0fd6d6e..92d3b37357d5 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1207,14 +1207,15 @@ bool __must_check try_grab_page(struct page *page, unsigned int flags); __maybe_unused 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); +/* + * This has the same functionality as try_get_compound_head(), just with a + * slightly different API. + */ 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; + return try_get_compound_head(page, 1) != NULL; } /** diff --git a/mm/gup.c b/mm/gup.c index 4be6f060fa0b..ba75906ba7f7 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -66,7 +66,7 @@ static void put_page_refs(struct page *page, int refs) * Return the compound head page with ref appropriately incremented, * or NULL if that failed. */ -static inline struct page *try_get_compound_head(struct page *page, int refs) +struct page *try_get_compound_head(struct page *page, int refs) { struct page *head = compound_head(page);
try_get_page() is very similar to try_get_compound_head(), and in fact try_get_page() has fallen a little behind in terms of maintenance: try_get_compound_head() handles speculative page references more thoroughly. Change try_get_page() so that it is implemented in terms of try_get_compound_head(), but without changing try_get_page()'s API contract. Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- include/linux/mm.h | 11 ++++++----- mm/gup.c | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-)