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

[Kent Overstreet <kent.overstreet@linux.dev>]

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
>