[PATCH v4 4/5] mm/filemap: add write_begin_get_folio() helper function

陈涛涛 Taotao Chen posted 5 patches 3 months ago
There is a newer version of this series
[PATCH v4 4/5] mm/filemap: add write_begin_get_folio() helper function
Posted by 陈涛涛 Taotao Chen 3 months ago
From: Taotao Chen <chentaotao@didiglobal.com>

Add write_begin_get_folio() to simplify the common folio lookup logic
used by filesystem ->write_begin() implementations.

This helper wraps __filemap_get_folio() with common flags such as
FGP_WRITEBEGIN, conditional FGP_DONTCACHE, and set folio order based
on the write length.

Part of a series refactoring address_space_operations write_begin and
write_end callbacks to use struct kiocb for passing write context and
flags.

Signed-off-by: Taotao Chen <chentaotao@didiglobal.com>
---
 include/linux/pagemap.h |  3 +++
 mm/filemap.c            | 30 ++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index e63fbfbd5b0f..cbf8539ba11b 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -749,6 +749,9 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
 		fgf_t fgp_flags, gfp_t gfp);
 struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index,
 		fgf_t fgp_flags, gfp_t gfp);
+struct folio *write_begin_get_folio(const struct kiocb *iocb,
+				    struct address_space *mapping,
+				    pgoff_t index, size_t len);
 
 /**
  * filemap_get_folio - Find and get a folio.
diff --git a/mm/filemap.c b/mm/filemap.c
index ba089d75fc86..9520f65c287a 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2026,6 +2026,36 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
 }
 EXPORT_SYMBOL(__filemap_get_folio);
 
+
+/**
+ * write_begin_get_folio - Get folio for write_begin with flags
+ * @iocb: kiocb passed from write_begin (may be NULL)
+ * @mapping: the address space to search in
+ * @index: page cache index
+ * @len: length of data being written
+ *
+ * This is a helper for filesystem write_begin() implementations.
+ * It wraps __filemap_get_folio(), setting appropriate flags in
+ * the write begin context.
+ *
+ * Returns a folio or an ERR_PTR.
+ */
+struct folio *write_begin_get_folio(const struct kiocb *iocb,
+				    struct address_space *mapping,
+				    pgoff_t index, size_t len)
+{
+	fgf_t fgp_flags = FGP_WRITEBEGIN;
+
+	fgp_flags |= fgf_set_order(len);
+
+	if (iocb && iocb->ki_flags & IOCB_DONTCACHE)
+		fgp_flags |= FGP_DONTCACHE;
+
+	return __filemap_get_folio(mapping, index, fgp_flags,
+				   mapping_gfp_mask(mapping));
+}
+EXPORT_SYMBOL(write_begin_get_folio);
+
 static inline struct folio *find_get_entry(struct xa_state *xas, pgoff_t max,
 		xa_mark_t mark)
 {
-- 
2.34.1
Re: [PATCH v4 4/5] mm/filemap: add write_begin_get_folio() helper function
Posted by Matthew Wilcox 3 months ago
On Mon, Jul 07, 2025 at 07:00:33AM +0000, 陈涛涛 Taotao Chen wrote:
> +++ b/mm/filemap.c

I think this should be a static inline function.  I don't think it's
worth moving out of line.  Of course if you have measurements that show
differently, you can change my mind.

> +/**
> + * write_begin_get_folio - Get folio for write_begin with flags
> + * @iocb: kiocb passed from write_begin (may be NULL)
> + * @mapping: the address space to search in
> + * @index: page cache index
> + * @len: length of data being written
> + *
> + * This is a helper for filesystem write_begin() implementations.
> + * It wraps __filemap_get_folio(), setting appropriate flags in
> + * the write begin context.
> + *
> + * Returns a folio or an ERR_PTR.

We prefer:

 * Return: A folio or an ERR_PTR

as this gets its own section in the kernel-doc output.

Re: [PATCH v4 4/5] mm/filemap: add write_begin_get_folio() helper function
Posted by Taotao Chen 3 months ago
在 2025/7/7 22:54, Matthew Wilcox 写道:
> On Mon, Jul 07, 2025 at 07:00:33AM +0000, 陈涛涛 Taotao Chen wrote:
>> +++ b/mm/filemap.c
> I think this should be a static inline function.  I don't think it's
> worth moving out of line.  Of course if you have measurements that show
> differently, you can change my mind.
>
>> +/**
>> + * write_begin_get_folio - Get folio for write_begin with flags
>> + * @iocb: kiocb passed from write_begin (may be NULL)
>> + * @mapping: the address space to search in
>> + * @index: page cache index
>> + * @len: length of data being written
>> + *
>> + * This is a helper for filesystem write_begin() implementations.
>> + * It wraps __filemap_get_folio(), setting appropriate flags in
>> + * the write begin context.
>> + *
>> + * Returns a folio or an ERR_PTR.
> We prefer:
>
>   * Return: A folio or an ERR_PTR
>
> as this gets its own section in the kernel-doc output.

Hi,

I’ll update both in the next version. Thanks for your review!

--Taotao



Re: [PATCH v4 4/5] mm/filemap: add write_begin_get_folio() helper function
Posted by hanqi 3 months ago

在 2025/7/7 15:00, 陈涛涛 Taotao Chen 写道:
> From: Taotao Chen <chentaotao@didiglobal.com>
>
> Add write_begin_get_folio() to simplify the common folio lookup logic
> used by filesystem ->write_begin() implementations.
>
> This helper wraps __filemap_get_folio() with common flags such as
> FGP_WRITEBEGIN, conditional FGP_DONTCACHE, and set folio order based
> on the write length.
>
> Part of a series refactoring address_space_operations write_begin and
> write_end callbacks to use struct kiocb for passing write context and
> flags.
>
> Signed-off-by: Taotao Chen <chentaotao@didiglobal.com>
> ---
>   include/linux/pagemap.h |  3 +++
>   mm/filemap.c            | 30 ++++++++++++++++++++++++++++++
>   2 files changed, 33 insertions(+)
>
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index e63fbfbd5b0f..cbf8539ba11b 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -749,6 +749,9 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
>   		fgf_t fgp_flags, gfp_t gfp);
>   struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index,
>   		fgf_t fgp_flags, gfp_t gfp);
> +struct folio *write_begin_get_folio(const struct kiocb *iocb,
> +				    struct address_space *mapping,
> +				    pgoff_t index, size_t len);
>   
>   /**
>    * filemap_get_folio - Find and get a folio.
> diff --git a/mm/filemap.c b/mm/filemap.c
> index ba089d75fc86..9520f65c287a 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2026,6 +2026,36 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
>   }
>   EXPORT_SYMBOL(__filemap_get_folio);
>   
> +
> +/**
> + * write_begin_get_folio - Get folio for write_begin with flags
> + * @iocb: kiocb passed from write_begin (may be NULL)
> + * @mapping: the address space to search in
> + * @index: page cache index
> + * @len: length of data being written
> + *
> + * This is a helper for filesystem write_begin() implementations.
> + * It wraps __filemap_get_folio(), setting appropriate flags in
> + * the write begin context.
> + *
> + * Returns a folio or an ERR_PTR.
> + */

hi, tao
I think it might be worth considering adding an fgf_t parameter to the
write_begin_get_folio() helper, since in some filesystems the fgp_flags
passed to __filemap_get_folio() in write_begin are not limited to just
FGP_WRITEBEGIN. Something like:
struct folio *write_begin_get_folio(const struct kiocb *iocb,
				    struct address_space *mapping,
				    pgoff_t index, size_t len,
                                     fgf_t fgp_flags)

> +struct folio *write_begin_get_folio(const struct kiocb *iocb,
> +				    struct address_space *mapping,
> +				    pgoff_t index, size_t len)
> +{
> +	fgf_t fgp_flags = FGP_WRITEBEGIN;
> +
> +	fgp_flags |= fgf_set_order(len);
> +
> +	if (iocb && iocb->ki_flags & IOCB_DONTCACHE)
> +		fgp_flags |= FGP_DONTCACHE;
> +
> +	return __filemap_get_folio(mapping, index, fgp_flags,
> +				   mapping_gfp_mask(mapping));
> +}
> +EXPORT_SYMBOL(write_begin_get_folio);
> +
>   static inline struct folio *find_get_entry(struct xa_state *xas, pgoff_t max,
>   		xa_mark_t mark)
>   {

Re: [PATCH v4 4/5] mm/filemap: add write_begin_get_folio() helper function
Posted by Matthew Wilcox 3 months ago
On Mon, Jul 07, 2025 at 07:48:34PM +0800, hanqi wrote:
> I think it might be worth considering adding an fgf_t parameter to the
> write_begin_get_folio() helper, since in some filesystems the fgp_flags
> passed to __filemap_get_folio() in write_begin are not limited to just
> FGP_WRITEBEGIN. Something like:
> struct folio *write_begin_get_folio(const struct kiocb *iocb,
> 				    struct address_space *mapping,
> 				    pgoff_t index, size_t len,
>                                     fgf_t fgp_flags)

The point is to make the simple cases simple.  Filesystems which need
something more complicated can do it all themselves.  NAK your idea.