Re: [PATCH 17/21] xfs: add file_{get,put}_folio

Kent Overstreet
2024-01-27T01:26:28Z

On Fri, Jan 26, 2024 at 02:28:59PM +0100, Christoph Hellwig wrote:
> From: "Darrick J. Wong" <djwong@kernel.org>
>
> Add helper similar to file_{get,set}_page, but which deal with folios
> and don't allocate new folio unless explicitly asked to, which map
> to shmem_get_folio instead of calling into the aops.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

looks boilerplatey to my eyes, but this is all new conceptual stuff and
the implementation will be evolving, so...

one nit: that's really not the right place for memalloc_nofs_save(), can
we try to start figuring out the proper locations for those?

Reviewed-by: Kent Overstreet <kent.overstreet@linux.dev>

> ---
> fs/xfs/scrub/trace.h | 2 ++
> fs/xfs/scrub/xfile.c | 74 ++++++++++++++++++++++++++++++++++++++++++++
> fs/xfs/scrub/xfile.h | 7 +++++
> 3 files changed, 83 insertions(+)
>
> diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
> index 0327cab606b070..c61fa7a95ef522 100644
> --- a/fs/xfs/scrub/trace.h
> +++ b/fs/xfs/scrub/trace.h
> @@ -908,6 +908,8 @@ DEFINE_XFILE_EVENT(xfile_store);
> DEFINE_XFILE_EVENT(xfile_seek_data);
> DEFINE_XFILE_EVENT(xfile_get_page);
> DEFINE_XFILE_EVENT(xfile_put_page);
> +DEFINE_XFILE_EVENT(xfile_get_folio);
> +DEFINE_XFILE_EVENT(xfile_put_folio);
>
> TRACE_EVENT(xfarray_create,
> TP_PROTO(struct xfarray *xfa, unsigned long long required_capacity),
> diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c
> index 2d802c20a8ddfe..1c1db4ae1ba6ee 100644
> --- a/fs/xfs/scrub/xfile.c
> +++ b/fs/xfs/scrub/xfile.c
> @@ -365,3 +365,77 @@ xfile_put_page(
> return -EIO;
> return 0;
> }
> +
> +/*
> + * Grab the (locked) folio for a memory object. The object cannot span a folio
> + * boundary. Returns the locked folio if successful, NULL if there was no
> + * folio or it didn't cover the range requested, or an ERR_PTR on failure.
> + */
> +struct folio *
> +xfile_get_folio(
> + struct xfile *xf,
> + loff_t pos,
> + size_t len,
> + unsigned int flags)
> +{
> + struct inode *inode = file_inode(xf->file);
> + struct folio *folio = NULL;
> + unsigned int pflags;
> + int error;
> +
> + if (inode->i_sb->s_maxbytes - pos < len)
> + return ERR_PTR(-ENOMEM);
> +
> + trace_xfile_get_folio(xf, pos, len);
> +
> + /*
> + * Increase the file size first so that shmem_get_folio(..., SGP_CACHE),
> + * actually allocates a folio instead of erroring out.
> + */
> + if ((flags & XFILE_ALLOC) && pos + len > i_size_read(inode))
> + i_size_write(inode, pos + len);
> +
> + pflags = memalloc_nofs_save();
> + error = shmem_get_folio(inode, pos >> PAGE_SHIFT, &folio,
> + (flags & XFILE_ALLOC) ? SGP_CACHE : SGP_READ);
> + memalloc_nofs_restore(pflags);
> + if (error)
> + return ERR_PTR(error);
> +
> + if (!folio)
> + return NULL;
> +
> + if (len > folio_size(folio) - offset_in_folio(folio, pos)) {
> + folio_unlock(folio);
> + folio_put(folio);
> + return NULL;
> + }
> +
> + if (xfile_has_lost_data(inode, folio)) {
> + folio_unlock(folio);
> + folio_put(folio);
> + return ERR_PTR(-EIO);
> + }
> +
> + /*
> + * Mark the folio dirty so that it won't be reclaimed once we drop the
> + * (potentially last) reference in xfile_put_folio.
> + */
> + if (flags & XFILE_ALLOC)
> + folio_set_dirty(folio);
> + return folio;
> +}
> +
> +/*
> + * Release the (locked) folio for a memory object.
> + */
> +void
> +xfile_put_folio(
> + struct xfile *xf,
> + struct folio *folio)
> +{
> + trace_xfile_put_folio(xf, folio_pos(folio), folio_size(folio));
> +
> + folio_unlock(folio);
> + folio_put(folio);
> +}
> diff --git a/fs/xfs/scrub/xfile.h b/fs/xfs/scrub/xfile.h
> index 465b10f492b66d..afb75e9fbaf265 100644
> --- a/fs/xfs/scrub/xfile.h
> +++ b/fs/xfs/scrub/xfile.h
> @@ -39,4 +39,11 @@ int xfile_get_page(struct xfile *xf, loff_t offset, unsigned int len,
> struct xfile_page *xbuf);
> int xfile_put_page(struct xfile *xf, struct xfile_page *xbuf);
>
> +#define XFILE_MAX_FOLIO_SIZE (PAGE_SIZE << MAX_PAGECACHE_ORDER)
> +
> +#define XFILE_ALLOC (1 << 0) /* allocate folio if not present */
> +struct folio *xfile_get_folio(struct xfile *xf, loff_t offset, size_t len,
> + unsigned int flags);
> +void xfile_put_folio(struct xfile *xf, struct folio *folio);
> +
> #endif /* __XFS_SCRUB_XFILE_H__ */
> --
> 2.39.2
>