This particular combination of flags is used by most filesystems
in their ->write_begin method, although it does find use in a
few other places. Before folios, it warranted its own function
(grab_cache_page_write_begin()), but I think that just having specialised
flags is enough. It certainly helps the few places that have been
converted from grab_cache_page_write_begin() to __filemap_get_folio().
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
fs/ext4/move_extent.c | 5 ++---
fs/iomap/buffered-io.c | 2 +-
fs/netfs/buffered_read.c | 3 +--
include/linux/pagemap.h | 2 ++
mm/folio-compat.c | 4 +---
5 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 2de9829aed63..0cb361f0a4fe 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -126,7 +126,6 @@ mext_folio_double_lock(struct inode *inode1, struct inode *inode2,
{
struct address_space *mapping[2];
unsigned int flags;
- unsigned fgp_flags = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE;
BUG_ON(!inode1 || !inode2);
if (inode1 < inode2) {
@@ -139,14 +138,14 @@ mext_folio_double_lock(struct inode *inode1, struct inode *inode2,
}
flags = memalloc_nofs_save();
- folio[0] = __filemap_get_folio(mapping[0], index1, fgp_flags,
+ folio[0] = __filemap_get_folio(mapping[0], index1, FGP_WRITEBEGIN,
mapping_gfp_mask(mapping[0]));
if (!folio[0]) {
memalloc_nofs_restore(flags);
return -ENOMEM;
}
- folio[1] = __filemap_get_folio(mapping[1], index2, fgp_flags,
+ folio[1] = __filemap_get_folio(mapping[1], index2, FGP_WRITEBEGIN,
mapping_gfp_mask(mapping[1]));
memalloc_nofs_restore(flags);
if (!folio[1]) {
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 6f4c97a6d7e9..10a203515583 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -467,7 +467,7 @@ EXPORT_SYMBOL_GPL(iomap_is_partially_uptodate);
*/
struct folio *iomap_get_folio(struct iomap_iter *iter, loff_t pos)
{
- unsigned fgp = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE | FGP_NOFS;
+ unsigned fgp = FGP_WRITEBEGIN | FGP_NOFS;
struct folio *folio;
if (iter->flags & IOMAP_NOWAIT)
diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c
index 7679a68e8193..e3d754a9e1b0 100644
--- a/fs/netfs/buffered_read.c
+++ b/fs/netfs/buffered_read.c
@@ -341,14 +341,13 @@ int netfs_write_begin(struct netfs_inode *ctx,
{
struct netfs_io_request *rreq;
struct folio *folio;
- unsigned int fgp_flags = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE;
pgoff_t index = pos >> PAGE_SHIFT;
int ret;
DEFINE_READAHEAD(ractl, file, NULL, mapping, index);
retry:
- folio = __filemap_get_folio(mapping, index, fgp_flags,
+ folio = __filemap_get_folio(mapping, index, FGP_WRITEBEGIN,
mapping_gfp_mask(mapping));
if (!folio)
return -ENOMEM;
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 9f1081683771..47069662f4b8 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -507,6 +507,8 @@ pgoff_t page_cache_prev_miss(struct address_space *mapping,
#define FGP_ENTRY 0x00000080
#define FGP_STABLE 0x00000100
+#define FGP_WRITEBEGIN (FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE)
+
struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
int fgp_flags, gfp_t gfp);
struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index,
diff --git a/mm/folio-compat.c b/mm/folio-compat.c
index 18c48b557926..668350748828 100644
--- a/mm/folio-compat.c
+++ b/mm/folio-compat.c
@@ -106,9 +106,7 @@ EXPORT_SYMBOL(pagecache_get_page);
struct page *grab_cache_page_write_begin(struct address_space *mapping,
pgoff_t index)
{
- unsigned fgp_flags = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE;
-
- return pagecache_get_page(mapping, index, fgp_flags,
+ return pagecache_get_page(mapping, index, FGP_WRITEBEGIN,
mapping_gfp_mask(mapping));
}
EXPORT_SYMBOL(grab_cache_page_write_begin);
--
2.35.1
"Matthew Wilcox (Oracle)" <[email protected]> writes:
> This particular combination of flags is used by most filesystems
> in their ->write_begin method, although it does find use in a
> few other places. Before folios, it warranted its own function
> (grab_cache_page_write_begin()), but I think that just having specialised
> flags is enough. It certainly helps the few places that have been
> converted from grab_cache_page_write_begin() to __filemap_get_folio().
>
> Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
Looks good to me. With small comment below.
Please feel free to add -
Reviewed-by: Ritesh Harjani (IBM) <[email protected]>
> ---
> fs/ext4/move_extent.c | 5 ++---
> fs/iomap/buffered-io.c | 2 +-
> fs/netfs/buffered_read.c | 3 +--
> include/linux/pagemap.h | 2 ++
> mm/folio-compat.c | 4 +---
> 5 files changed, 7 insertions(+), 9 deletions(-)
After below patch got added to mainline, we should use FGP_WRITEBEGIN
flag in fs/nfs/file.c as well.
54d99381b7371d2999566d1fb4ea88d46cf9d865
Author: Trond Myklebust <[email protected]>
CommitDate: Tue Feb 14 14:22:32 2023 -0500
NFS: Convert nfs_write_begin/end to use folios
In fact we don't even need the helper
(nfs_folio_grab_cache_write_begin()) anymore, since we can directly pass
FGP_WRITEBEGIN flag in __filemap_get_folio() in the caller itself.
static struct folio *
nfs_folio_grab_cache_write_begin(struct address_space *mapping, pgoff_t index)
{
unsigned fgp_flags = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE;
return __filemap_get_folio(mapping, index, fgp_flags,
mapping_gfp_mask(mapping));
}
-ritesh
>
> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
> index 2de9829aed63..0cb361f0a4fe 100644
> --- a/fs/ext4/move_extent.c
> +++ b/fs/ext4/move_extent.c
> @@ -126,7 +126,6 @@ mext_folio_double_lock(struct inode *inode1, struct inode *inode2,
> {
> struct address_space *mapping[2];
> unsigned int flags;
> - unsigned fgp_flags = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE;
>
> BUG_ON(!inode1 || !inode2);
> if (inode1 < inode2) {
> @@ -139,14 +138,14 @@ mext_folio_double_lock(struct inode *inode1, struct inode *inode2,
> }
>
> flags = memalloc_nofs_save();
> - folio[0] = __filemap_get_folio(mapping[0], index1, fgp_flags,
> + folio[0] = __filemap_get_folio(mapping[0], index1, FGP_WRITEBEGIN,
> mapping_gfp_mask(mapping[0]));
> if (!folio[0]) {
> memalloc_nofs_restore(flags);
> return -ENOMEM;
> }
>
> - folio[1] = __filemap_get_folio(mapping[1], index2, fgp_flags,
> + folio[1] = __filemap_get_folio(mapping[1], index2, FGP_WRITEBEGIN,
> mapping_gfp_mask(mapping[1]));
> memalloc_nofs_restore(flags);
> if (!folio[1]) {
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 6f4c97a6d7e9..10a203515583 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -467,7 +467,7 @@ EXPORT_SYMBOL_GPL(iomap_is_partially_uptodate);
> */
> struct folio *iomap_get_folio(struct iomap_iter *iter, loff_t pos)
> {
> - unsigned fgp = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE | FGP_NOFS;
> + unsigned fgp = FGP_WRITEBEGIN | FGP_NOFS;
> struct folio *folio;
>
> if (iter->flags & IOMAP_NOWAIT)
> diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c
> index 7679a68e8193..e3d754a9e1b0 100644
> --- a/fs/netfs/buffered_read.c
> +++ b/fs/netfs/buffered_read.c
> @@ -341,14 +341,13 @@ int netfs_write_begin(struct netfs_inode *ctx,
> {
> struct netfs_io_request *rreq;
> struct folio *folio;
> - unsigned int fgp_flags = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE;
> pgoff_t index = pos >> PAGE_SHIFT;
> int ret;
>
> DEFINE_READAHEAD(ractl, file, NULL, mapping, index);
>
> retry:
> - folio = __filemap_get_folio(mapping, index, fgp_flags,
> + folio = __filemap_get_folio(mapping, index, FGP_WRITEBEGIN,
> mapping_gfp_mask(mapping));
> if (!folio)
> return -ENOMEM;
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 9f1081683771..47069662f4b8 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -507,6 +507,8 @@ pgoff_t page_cache_prev_miss(struct address_space *mapping,
> #define FGP_ENTRY 0x00000080
> #define FGP_STABLE 0x00000100
>
> +#define FGP_WRITEBEGIN (FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE)
> +
> struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
> int fgp_flags, gfp_t gfp);
> struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index,
> diff --git a/mm/folio-compat.c b/mm/folio-compat.c
> index 18c48b557926..668350748828 100644
> --- a/mm/folio-compat.c
> +++ b/mm/folio-compat.c
> @@ -106,9 +106,7 @@ EXPORT_SYMBOL(pagecache_get_page);
> struct page *grab_cache_page_write_begin(struct address_space *mapping,
> pgoff_t index)
> {
> - unsigned fgp_flags = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE;
> -
> - return pagecache_get_page(mapping, index, fgp_flags,
> + return pagecache_get_page(mapping, index, FGP_WRITEBEGIN,
> mapping_gfp_mask(mapping));
> }
> EXPORT_SYMBOL(grab_cache_page_write_begin);
> --
> 2.35.1
On Thu, Jan 26, 2023 at 08:23:45PM +0000, Matthew Wilcox (Oracle) wrote:
> This particular combination of flags is used by most filesystems
> in their ->write_begin method, although it does find use in a
> few other places. Before folios, it warranted its own function
> (grab_cache_page_write_begin()), but I think that just having specialised
> flags is enough. It certainly helps the few places that have been
> converted from grab_cache_page_write_begin() to __filemap_get_folio().
>
> Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
Reviewed-by: Theodore Ts'o <[email protected]>