| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 10 Jan 2022 21:00:41 -0800
From: John Hubbard <jhubbard@...dia.com>
To: "Matthew Wilcox (Oracle)" <willy@...radead.org>, linux-mm@...ck.org
Cc: Christoph Hellwig <hch@...radead.org>,
William Kucharski <william.kucharski@...cle.com>,
linux-kernel@...r.kernel.org, Jason Gunthorpe <jgg@...pe.ca>
Subject: Re: [PATCH v2 15/28] gup: Add try_get_folio() and try_grab_folio()
On 1/9/22 20:23, Matthew Wilcox (Oracle) wrote:
> Convert try_get_compound_head() into try_get_folio() and convert
> try_grab_compound_head() into try_grab_folio(). Also convert
> hpage_pincount_add() to folio_pincount_add(). Add a temporary
> try_grab_compound_head() wrapper around try_grab_folio() to
> let us convert callers individually.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@...radead.org>
> ---
> mm/gup.c | 104 +++++++++++++++++++++++++-------------------------
> mm/internal.h | 5 +++
> 2 files changed, 56 insertions(+), 53 deletions(-)
Still looks good.
Reviewed-by: John Hubbard <jhubbard@...dia.com>
thanks,
--
John Hubbard
NVIDIA
>
> diff --git a/mm/gup.c b/mm/gup.c
> index 1282d29357b7..9e581201d679 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -30,21 +30,19 @@ struct follow_page_context {
> };
>
> /*
> - * When pinning a compound page, use an exact count to track it, via
> - * page_pincount_add/_sub().
> + * When pinning a large folio, use an exact count to track it.
> *
> - * However, be sure to *also* increment the normal page refcount field
> - * at least once, so that the page really is pinned. That's why the
> - * refcount from the earlier try_get_compound_head() is left intact.
> + * However, be sure to *also* increment the normal folio refcount
> + * field at least once, so that the folio really is pinned.
> + * That's why the refcount from the earlier
> + * try_get_folio() is left intact.
> */
> -static void page_pincount_add(struct page *page, int refs)
> +static void folio_pincount_add(struct folio *folio, int refs)
> {
> - VM_BUG_ON_PAGE(page != compound_head(page), page);
> -
> - if (PageHead(page))
> - atomic_add(refs, compound_pincount_ptr(page));
> + if (folio_test_large(folio))
> + atomic_add(refs, folio_pincount_ptr(folio));
> else
> - page_ref_add(page, refs * (GUP_PIN_COUNTING_BIAS - 1));
> + folio_ref_add(folio, refs * (GUP_PIN_COUNTING_BIAS - 1));
> }
>
> static int page_pincount_sub(struct page *page, int refs)
> @@ -76,75 +74,70 @@ static void put_page_refs(struct page *page, int refs)
> }
>
> /*
> - * Return the compound head page with ref appropriately incremented,
> + * Return the folio with ref appropriately incremented,
> * or NULL if that failed.
> */
> -static inline struct page *try_get_compound_head(struct page *page, int refs)
> +static inline struct folio *try_get_folio(struct page *page, int refs)
> {
> - struct page *head;
> + struct folio *folio;
>
> retry:
> - head = compound_head(page);
> -
> - if (WARN_ON_ONCE(page_ref_count(head) < 0))
> + folio = page_folio(page);
> + if (WARN_ON_ONCE(folio_ref_count(folio) < 0))
> return NULL;
> - if (unlikely(!page_cache_add_speculative(head, refs)))
> + if (unlikely(!folio_ref_try_add_rcu(folio, refs)))
> return NULL;
>
> /*
> - * At this point we have a stable reference to the head page; but it
> - * could be that between the compound_head() lookup and the refcount
> - * increment, the compound page was split, in which case we'd end up
> - * holding a reference on a page that has nothing to do with the page
> + * At this point we have a stable reference to the folio; but it
> + * could be that between calling page_folio() and the refcount
> + * increment, the folio was split, in which case we'd end up
> + * holding a reference on a folio that has nothing to do with the page
> * we were given anymore.
> - * So now that the head page is stable, recheck that the pages still
> - * belong together.
> + * So now that the folio is stable, recheck that the page still
> + * belongs to this folio.
> */
> - if (unlikely(compound_head(page) != head)) {
> - put_page_refs(head, refs);
> + if (unlikely(page_folio(page) != folio)) {
> + folio_put_refs(folio, refs);
> goto retry;
> }
>
> - return head;
> + return folio;
> }
>
> /**
> - * try_grab_compound_head() - attempt to elevate a page's refcount, by a
> - * flags-dependent amount.
> - *
> - * Even though the name includes "compound_head", this function is still
> - * appropriate for callers that have a non-compound @page to get.
> - *
> + * try_grab_folio() - Attempt to get or pin a folio.
> * @page: pointer to page to be grabbed
> - * @refs: the value to (effectively) add to the page's refcount
> + * @refs: the value to (effectively) add to the folio's refcount
> * @flags: gup flags: these are the FOLL_* flag values.
> *
> * "grab" names in this file mean, "look at flags to decide whether to use
> - * FOLL_PIN or FOLL_GET behavior, when incrementing the page's refcount.
> + * FOLL_PIN or FOLL_GET behavior, when incrementing the folio's refcount.
> *
> * Either FOLL_PIN or FOLL_GET (or neither) must be set, but not both at the
> * same time. (That's true throughout the get_user_pages*() and
> * pin_user_pages*() APIs.) Cases:
> *
> - * FOLL_GET: page's refcount will be incremented by @refs.
> + * FOLL_GET: folio's refcount will be incremented by @refs.
> *
> - * FOLL_PIN on compound pages: page's refcount will be incremented by
> - * @refs, and page[1].compound_pincount will be incremented by @refs.
> + * FOLL_PIN on large folios: folio's refcount will be incremented by
> + * @refs, and its compound_pincount will be incremented by @refs.
> *
> - * FOLL_PIN on normal pages: page's refcount will be incremented by
> + * FOLL_PIN on single-page folios: folio's refcount will be incremented by
> * @refs * GUP_PIN_COUNTING_BIAS.
> *
> - * Return: head page (with refcount appropriately incremented) for success, or
> - * NULL upon failure. If neither FOLL_GET nor FOLL_PIN was set, that's
> - * considered failure, and furthermore, a likely bug in the caller, so a warning
> - * is also emitted.
> + * Return: The folio containing @page (with refcount appropriately
> + * incremented) for success, or NULL upon failure. If neither FOLL_GET
> + * nor FOLL_PIN was set, that's considered failure, and furthermore,
> + * a likely bug in the caller, so a warning is also emitted.
> */
> -struct page *try_grab_compound_head(struct page *page,
> - int refs, unsigned int flags)
> +struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags)
> {
> if (flags & FOLL_GET)
> - return try_get_compound_head(page, refs);
> + return try_get_folio(page, refs);
> else if (flags & FOLL_PIN) {
> + struct folio *folio;
> +
> /*
> * Can't do FOLL_LONGTERM + FOLL_PIN gup fast path if not in a
> * right zone, so fail and let the caller fall back to the slow
> @@ -158,21 +151,26 @@ struct page *try_grab_compound_head(struct page *page,
> * CAUTION: Don't use compound_head() on the page before this
> * point, the result won't be stable.
> */
> - page = try_get_compound_head(page, refs);
> - if (!page)
> + folio = try_get_folio(page, refs);
> + if (!folio)
> return NULL;
>
> - page_pincount_add(page, refs);
> - mod_node_page_state(page_pgdat(page), NR_FOLL_PIN_ACQUIRED,
> - refs);
> + folio_pincount_add(folio, refs);
> + node_stat_mod_folio(folio, NR_FOLL_PIN_ACQUIRED, refs);
>
> - return page;
> + return folio;
> }
>
> WARN_ON_ONCE(1);
> return NULL;
> }
>
> +struct page *try_grab_compound_head(struct page *page,
> + int refs, unsigned int flags)
> +{
> + return &try_grab_folio(page, refs, flags)->page;
> +}
> +
> static void put_compound_head(struct page *page, int refs, unsigned int flags)
> {
> if (flags & FOLL_PIN) {
> @@ -196,7 +194,7 @@ static void put_compound_head(struct page *page, int refs, unsigned int flags)
> * @flags: gup flags: these are the FOLL_* flag values.
> *
> * Either FOLL_PIN or FOLL_GET (or neither) may be set, but not both at the same
> - * time. Cases: please see the try_grab_compound_head() documentation, with
> + * time. Cases: please see the try_grab_folio() documentation, with
> * "refs=1".
> *
> * Return: true for success, or if no action was required (if neither FOLL_PIN
> diff --git a/mm/internal.h b/mm/internal.h
> index 26af8a5a5be3..9a72d1ecdab4 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -723,4 +723,9 @@ void vunmap_range_noflush(unsigned long start, unsigned long end);
> int numa_migrate_prep(struct page *page, struct vm_area_struct *vma,
> unsigned long addr, int page_nid, int *flags);
>
> +/*
> + * mm/gup.c
> + */
> +struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags);
> +
> #endif /* __MM_INTERNAL_H */
Powered by blists - more mailing lists