On Fri, Oct 14, 2022 at 04:57:55PM -0700, Dan Williams wrote:
> +/**
> + * pgmap_request_folios - activate an contiguous span of folios in @pgmap
> + * @pgmap: host page map for the folio array
> + * @folio: start of the folio list, all subsequent folios have same folio_size()
> + *
> + * Caller is responsible for @pgmap remaining live for the duration of
> + * this call. Caller is also responsible for not racing requests for the
> + * same folios.
> + */
> +bool pgmap_request_folios(struct dev_pagemap *pgmap, struct folio *folio,
> + int nr_folios)
> +{
> + struct folio *iter;
> + int i;
> +
> + /*
> + * All of the WARNs below are for catching bugs in future
> + * development that changes the assumptions of:
> + * 1/ uniform folios in @pgmap
> + * 2/ @pgmap death does not race this routine.
> + */
> + VM_WARN_ON_ONCE(!folio_span_valid(pgmap, folio, nr_folios));
> +
> + if (WARN_ON_ONCE(percpu_ref_is_dying(&pgmap->ref)))
> + return false;
> +
> + for (iter = folio_next(folio), i = 1; i < nr_folios;
> + iter = folio_next(folio), i++)
> + if (WARN_ON_ONCE(folio_order(iter) != folio_order(folio)))
> + return false;
> +
> + for (iter = folio, i = 0; i < nr_folios; iter = folio_next(iter), i++) {
> + folio_ref_inc(iter);
> + if (folio_ref_count(iter) == 1)
This seems strange, I would say if the folio doesn't have 0 ref at
this point it is a caller bug. The caller should only be able to
"request" folios once.
Ie the caller should not attempt to request a folio until it has been
released back via free page.
>From a FS viewpoint this makes sense as if we establish a new inode
then those pages under the inode had better be currently exclusive to
the inode, or we have some rouge entity touching them.
As is, this looks racy because if you assume transient refs are now
allowed then their could be a 2->1 transition immediately after
ref_inc.
Having a release operation seems unworkable for a refcounted structure:
> +void pgmap_release_folios(struct dev_pagemap *pgmap, struct folio *folio, int nr_folios)
> +{
> + struct folio *iter;
> + int i;
> +
> + for (iter = folio, i = 0; i < nr_folios; iter = folio_next(iter), i++) {
> + if (!put_devmap_managed_page(&iter->page))
> + folio_put(iter);
> + if (!folio_ref_count(iter))
> + put_dev_pagemap(pgmap);
As if we don't have a 0 refcount here (eg due to transient external
page ref) we have now lost the put_dev_pagemap()
Jason
