[RFC PATCH 0/8] fat: convert to iomap

[RFC PATCH 0/8] fat: convert to iomap

[Matheus Alves de Almeida]

Hi,

This series converts fat to use iomap. It follows roughly the direction
suggested by https://docs.kernel.org/filesystems/iomap/porting.html,
with the exfat conversion series and related fixes as a reference.

Overall, the largest performance improvements were seen for extending
writes (~30%) and fallocate() (~18%).

Thanks,
Matheus

Matheus Alves de Almeida (8):
  fat: support FIEMAP
  fat: use iomap for ->bmap()
  fat: add iomap direct I/O support
  fat: clean stale bdev buffers of newly allocated clusters
  fat: drop ->write_begin() and ->write_end() usage from
    fat_cont_expand()
  fat: add iomap buffered I/O support
  fat: remove ->mmu_private
  fat: add multi-cluster allocation for write

 fs/fat/Kconfig  |   2 +-
 fs/fat/Makefile |   2 +-
 fs/fat/cache.c  |  95 ++++++++++-----
 fs/fat/dir.c    |   1 -
 fs/fat/fat.h    |  20 +--
 fs/fat/fatent.c |  67 ++++++++--
 fs/fat/file.c   | 316 ++++++++++++++++++++++++++++++++++++++----------
 fs/fat/inode.c  | 248 +++++--------------------------------
 fs/fat/iomap.c  | 180 +++++++++++++++++++++++++++
 9 files changed, 599 insertions(+), 332 deletions(-)
 create mode 100644 fs/fat/iomap.c

-- 
2.43.0

[RFC PATCH 1/8] fat: support FIEMAP

[Matheus Alves de Almeida]

Implement support for ->fiemap() using iomap_fiemap(). Add a read-only
fat_iomap_begin() and a fat_get_cont_clusters() helper for finding
contiguous clusters.

Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
---
 fs/fat/Kconfig  |  1 +
 fs/fat/Makefile |  2 +-
 fs/fat/cache.c  | 80 ++++++++++++++++++++++++++++++++++++++-----------
 fs/fat/fat.h    |  5 ++++
 fs/fat/file.c   | 12 ++++++++
 fs/fat/iomap.c  | 71 +++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 152 insertions(+), 19 deletions(-)
 create mode 100644 fs/fat/iomap.c

diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
index 25fae1c83725..d31d7866ce19 100644
--- a/fs/fat/Kconfig
+++ b/fs/fat/Kconfig
@@ -2,6 +2,7 @@
 config FAT_FS
 	tristate
 	select BUFFER_HEAD
+	select FS_IOMAP
 	select NLS
 	select LEGACY_DIRECT_IO
 	help
diff --git a/fs/fat/Makefile b/fs/fat/Makefile
index 2b034112690d..238668f03744 100644
--- a/fs/fat/Makefile
+++ b/fs/fat/Makefile
@@ -7,7 +7,7 @@ obj-$(CONFIG_FAT_FS) += fat.o
 obj-$(CONFIG_VFAT_FS) += vfat.o
 obj-$(CONFIG_MSDOS_FS) += msdos.o
 
-fat-y := cache.o dir.o fatent.o file.o inode.o misc.o nfs.o
+fat-y := cache.o dir.o fatent.o file.o inode.o misc.o nfs.o iomap.o
 vfat-y := namei_vfat.o
 msdos-y := namei_msdos.o
 
diff --git a/fs/fat/cache.c b/fs/fat/cache.c
index 1b87354e24ba..f545e1db1d47 100644
--- a/fs/fat/cache.c
+++ b/fs/fat/cache.c
@@ -76,7 +76,7 @@ static int fat_cache_lookup(struct inode *inode, int fclus,
 			    struct fat_cache_id *cid,
 			    int *cached_fclus, int *cached_dclus)
 {
-	static struct fat_cache nohit = { .fcluster = 0, };
+	static struct fat_cache nohit = { .fcluster = -1, };
 
 	struct fat_cache *hit = &nohit, *p;
 	int offset = -1;
@@ -217,14 +217,16 @@ static inline void cache_init(struct fat_cache_id *cid, int fclus, int dclus)
 	cid->nr_contig = 0;
 }
 
-int fat_get_cluster(struct inode *inode, int cluster, int *fclus, int *dclus)
+static int __fat_get_cluster(struct inode *inode, int cluster, int *fclus,
+			     int *dclus, int *nr_cont)
 {
 	struct super_block *sb = inode->i_sb;
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 	const int limit = sb->s_maxbytes >> sbi->cluster_bits;
 	struct fat_entry fatent;
 	struct fat_cache_id cid;
-	int nr;
+	int last = nr_cont ? cluster + *nr_cont - 1 : cluster;
+	int fcluster, dcluster, nr;
 
 	BUG_ON(MSDOS_I(inode)->i_start == 0);
 
@@ -236,21 +238,27 @@ int fat_get_cluster(struct inode *inode, int cluster, int *fclus, int *dclus)
 			__func__, MSDOS_I(inode)->i_pos, *dclus);
 		return -EIO;
 	}
-	if (cluster == 0)
+	if (last == 0)
 		return 0;
 
 	if (fat_cache_lookup(inode, cluster, &cid, fclus, dclus) < 0) {
-		/*
-		 * dummy, always not contiguous
-		 * This is reinitialized by cache_init(), later.
-		 */
-		cache_init(&cid, -1, -1);
+		/* Cache the contiguous prefix from the known first cluster. */
+		cache_init(&cid, 0, *dclus);
 	}
 
+	fcluster = *fclus;
+	dcluster = *dclus;
 	fatent_init(&fatent);
-	while (*fclus < cluster) {
+	while (fcluster < last) {
+		if (nr_cont && fcluster == cluster && cid.fcluster != -1) {
+			fcluster = min(last, cid.fcluster + cid.nr_contig);
+			dcluster = cid.dcluster + fcluster - cid.fcluster;
+			if (fcluster == last)
+				break;
+		}
+
 		/* prevent the infinite loop of cluster chain */
-		if (*fclus > limit) {
+		if (fcluster > limit) {
 			fat_fs_error_ratelimit(sb,
 				"%s: detected the cluster chain loop (i_pos %lld)",
 				__func__, MSDOS_I(inode)->i_pos);
@@ -258,31 +266,67 @@ int fat_get_cluster(struct inode *inode, int cluster, int *fclus, int *dclus)
 			goto out;
 		}
 
-		nr = fat_ent_read(inode, &fatent, *dclus);
+		nr = fat_ent_read(inode, &fatent, dcluster);
 		if (nr < 0)
 			goto out;
+		else if (nr == FAT_ENT_EOF)
+			goto cache_add;
 		else if (nr == FAT_ENT_FREE) {
 			fat_fs_error_ratelimit(sb,
 				"%s: invalid cluster chain (i_pos %lld)",
 				__func__, MSDOS_I(inode)->i_pos);
 			nr = -EIO;
 			goto out;
-		} else if (nr == FAT_ENT_EOF) {
+		}
+		if (nr_cont && fcluster >= cluster && nr != dcluster + 1) {
 			fat_cache_add(inode, &cid);
-			goto out;
+			cache_init(&cid, fcluster + 1, nr);
+			break;
+		}
+		fcluster++;
+		dcluster = nr;
+		if (!cache_contiguous(&cid, dcluster))
+			cache_init(&cid, fcluster, dcluster);
+		if (fcluster <= cluster) {
+			*fclus = fcluster;
+			*dclus = dcluster;
 		}
-		(*fclus)++;
-		*dclus = nr;
-		if (!cache_contiguous(&cid, *dclus))
-			cache_init(&cid, *fclus, *dclus);
 	}
 	nr = 0;
+	if (nr_cont)
+		*nr_cont = fcluster - cluster + 1;
+cache_add:
 	fat_cache_add(inode, &cid);
 out:
 	fatent_brelse(&fatent);
 	return nr;
 }
 
+int fat_get_cluster(struct inode *inode, int cluster, int *fclus, int *dclus)
+{
+	return __fat_get_cluster(inode, cluster, fclus, dclus, NULL);
+}
+
+int fat_get_cont_clusters(struct inode *inode, int cluster, int *nr)
+{
+	int ret, fclus, dclus;
+
+	if (!MSDOS_I(inode)->i_start)
+		return -EIO;
+
+	ret = __fat_get_cluster(inode, cluster, &fclus, &dclus, nr);
+	if (ret < 0)
+		return ret;
+	if (ret == FAT_ENT_EOF) {
+		fat_fs_error(inode->i_sb,
+			     "%s: request beyond EOF (i_pos %lld)",
+			     __func__, MSDOS_I(inode)->i_pos);
+		return -EIO;
+	}
+	return dclus;
+}
+
+
 static int fat_bmap_cluster(struct inode *inode, int cluster)
 {
 	struct super_block *sb = inode->i_sb;
diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 204c7b87108f..3d0f91fa11df 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -3,6 +3,7 @@
 #define _FAT_H
 
 #include <linux/buffer_head.h>
+#include <linux/iomap.h>
 #include <linux/nls.h>
 #include <linux/hash.h>
 #include <linux/ratelimit.h>
@@ -321,6 +322,7 @@ static inline void fatwchar_to16(__u8 *dst, const wchar_t *src, size_t len)
 extern void fat_cache_inval_inode(struct inode *inode);
 extern int fat_get_cluster(struct inode *inode, int cluster,
 			   int *fclus, int *dclus);
+int fat_get_cont_clusters(struct inode *inode, int cluster, int *nr);
 extern int fat_get_mapped_cluster(struct inode *inode, sector_t sector,
 				  sector_t last_block,
 				  unsigned long *mapped_blocks, sector_t *bmap);
@@ -488,6 +490,9 @@ void fat_cache_destroy(void);
 extern const struct export_operations fat_export_ops;
 extern const struct export_operations fat_export_ops_nostale;
 
+/* fat/iomap.c */
+extern const struct iomap_ops fat_iomap_ops;
+
 /* helper for printk */
 typedef unsigned long long	llu;
 
diff --git a/fs/fat/file.c b/fs/fat/file.c
index adc5554eeab6..531abf9e533c 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -636,9 +636,21 @@ int fat_setattr(const struct mnt_idmap *idmap, struct dentry *dentry,
 }
 EXPORT_SYMBOL_GPL(fat_setattr);
 
+static int fat_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
+		      u64 start, u64 len)
+{
+	int err;
+
+	inode_lock_shared(inode);
+	err = iomap_fiemap(inode, fieinfo, start, len, &fat_iomap_ops);
+	inode_unlock_shared(inode);
+	return err;
+}
+
 const struct inode_operations fat_file_inode_operations = {
 	.setattr	= fat_setattr,
 	.getattr	= fat_getattr,
 	.fileattr_get	= fat_fileattr_get,
 	.update_time	= fat_update_time,
+	.fiemap		= fat_fiemap,
 };
diff --git a/fs/fat/iomap.c b/fs/fat/iomap.c
new file mode 100644
index 000000000000..7fb24d0296f7
--- /dev/null
+++ b/fs/fat/iomap.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * FAT iomap support
+ *
+ * Copyright (C) 2026 Matheus Alves de Almeida
+ */
+
+#include "fat.h"
+
+static int __fat_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+			     unsigned int flags, struct iomap *iomap)
+{
+	struct super_block *sb = inode->i_sb;
+	struct msdos_sb_info *sbi = MSDOS_SB(sb);
+	int start_cluster, end_cluster, nr_cluster, ret;
+	loff_t curr_size = i_size_read(inode);
+	loff_t aligned_offset;
+
+	iomap->flags = 0;
+	/* Return IOMAP_HOLE if we are beyond any kind of disk allocation */
+	if (inode_get_bytes(inode) <= offset) {
+		iomap->addr = IOMAP_NULL_ADDR;
+		iomap->offset = offset;
+		iomap->length = length;
+		iomap->type = IOMAP_HOLE;
+		return 0;
+	}
+
+	/*
+	 * We need to separate normal data clusters and
+	 * uninitialized FALLOC_FL_KEEP_SIZE clusters.
+	 */
+	start_cluster = offset >> sbi->cluster_bits;
+	aligned_offset = (u64)start_cluster << sbi->cluster_bits;
+	if (curr_size <= aligned_offset) {
+		iomap->type = IOMAP_UNWRITTEN;
+		length = min(offset - aligned_offset + length,
+			     inode_get_bytes(inode) - aligned_offset);
+	} else {
+		iomap->type = IOMAP_MAPPED;
+		length = min(offset - aligned_offset + length,
+			     curr_size - aligned_offset);
+	}
+
+	end_cluster = (aligned_offset + length - 1) >> sbi->cluster_bits;
+	nr_cluster = end_cluster - start_cluster + 1;
+	ret = fat_get_cont_clusters(inode, start_cluster, &nr_cluster);
+	if (ret < 0)
+		return ret;
+
+	iomap->addr = (u64)fat_clus_to_blknr(sbi, ret) << sb->s_blocksize_bits;
+	iomap->offset = aligned_offset;
+	iomap->length = (u64)nr_cluster << sbi->cluster_bits;
+	iomap->flags |= IOMAP_F_MERGED;
+	iomap->bdev = sb->s_bdev;
+
+	return 0;
+}
+
+static int fat_iomap_begin(struct inode *inode, loff_t offset,
+			   loff_t length, unsigned int flags,
+			   struct iomap *iomap, struct iomap *srcmap)
+{
+	return __fat_iomap_begin(inode, offset, length, flags, iomap);
+}
+
+static DEFINE_IOMAP_ITER_NEXT(fat_iomap_next, fat_iomap_begin);
+
+const struct iomap_ops fat_iomap_ops = {
+	.iomap_next = fat_iomap_next
+};
-- 
2.43.0

Re: [RFC PATCH 1/8] fat: support FIEMAP

[OGAWA Hirofumi]

Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br> writes:

> Implement support for ->fiemap() using iomap_fiemap(). Add a read-only
> fat_iomap_begin() and a fat_get_cont_clusters() helper for finding
> contiguous clusters.

Not fully reviewed and I'm not understanding iomap fully though.

> Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
> ---
>  fs/fat/Kconfig  |  1 +
>  fs/fat/Makefile |  2 +-
>  fs/fat/cache.c  | 80 ++++++++++++++++++++++++++++++++++++++-----------
>  fs/fat/fat.h    |  5 ++++
>  fs/fat/file.c   | 12 ++++++++
>  fs/fat/iomap.c  | 71 +++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 152 insertions(+), 19 deletions(-)
>  create mode 100644 fs/fat/iomap.c
>

[...]

> diff --git a/fs/fat/cache.c b/fs/fat/cache.c
> index 1b87354e24ba..f545e1db1d47 100644
> --- a/fs/fat/cache.c
> +++ b/fs/fat/cache.c
> @@ -76,7 +76,7 @@ static int fat_cache_lookup(struct inode *inode, int fclus,
>  			    struct fat_cache_id *cid,
>  			    int *cached_fclus, int *cached_dclus)
>  {
> -	static struct fat_cache nohit = { .fcluster = 0, };
> +	static struct fat_cache nohit = { .fcluster = -1, };

Maybe this change is pointless?

>  	struct fat_cache *hit = &nohit, *p;
>  	int offset = -1;
> @@ -217,14 +217,16 @@ static inline void cache_init(struct fat_cache_id *cid, int fclus, int dclus)
>  	cid->nr_contig = 0;
>  }
>  
> -int fat_get_cluster(struct inode *inode, int cluster, int *fclus, int *dclus)
> +static int __fat_get_cluster(struct inode *inode, int cluster, int *fclus,
> +			     int *dclus, int *nr_cont)
>  {
>  	struct super_block *sb = inode->i_sb;
>  	struct msdos_sb_info *sbi = MSDOS_SB(sb);
>  	const int limit = sb->s_maxbytes >> sbi->cluster_bits;
>  	struct fat_entry fatent;
>  	struct fat_cache_id cid;

[...]

>  
>  	if (fat_cache_lookup(inode, cluster, &cid, fclus, dclus) < 0) {
> -		/*
> -		 * dummy, always not contiguous
> -		 * This is reinitialized by cache_init(), later.
> -		 */
> -		cache_init(&cid, -1, -1);
> +		/* Cache the contiguous prefix from the known first cluster. */
> +		cache_init(&cid, 0, *dclus);

Doesn't this add a needless cache? 0 is cached in the inode itself,
isn't it? (and can be waste small fatent cache slots?)

>  	}
>  
> +	fcluster = *fclus;
> +	dcluster = *dclus;
>  	fatent_init(&fatent);
> -	while (*fclus < cluster) {
> +	while (fcluster < last) {
> +		if (nr_cont && fcluster == cluster && cid.fcluster != -1) {

cid.fcluster == -1 is possible with this patch?

> +			fcluster = min(last, cid.fcluster + cid.nr_contig);
> +			dcluster = cid.dcluster + fcluster - cid.fcluster;
> +			if (fcluster == last)
> +				break;
> +		}
> +

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[RFC PATCH 2/8] fat: use iomap for ->bmap()

[Matheus Alves de Almeida]

Make FAT ->bmap() use iomap_bmap(). Add an iomap-backed
fat_swap_activate() to prevent a swapon() deadlock. Remove
truncate_lock as ->bmap() now uses i_rwsem.

Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
---
 fs/fat/fat.h   |  2 +-
 fs/fat/file.c  |  2 --
 fs/fat/inode.c | 47 ++++++++++++++---------------------------------
 fs/fat/iomap.c | 24 ++++++++++++++++++++++++
 4 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 3d0f91fa11df..81c454f8a78a 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -131,7 +131,6 @@ struct msdos_inode_info {
 	loff_t i_pos;		/* on-disk position of directory entry or 0 */
 	struct hlist_node i_fat_hash;	/* hash by i_location */
 	struct hlist_node i_dir_hash;	/* hash by i_logstart */
-	struct rw_semaphore truncate_lock; /* protect bmap against truncate */
 	struct timespec64 i_crtime;	/* File creation (birth) time */
 	struct mapping_metadata_bhs i_metadata_bhs;
 	struct inode vfs_inode;
@@ -492,6 +491,7 @@ extern const struct export_operations fat_export_ops_nostale;
 
 /* fat/iomap.c */
 extern const struct iomap_ops fat_iomap_ops;
+extern const struct iomap_ops fat_iomap_bmap_ops;
 
 /* helper for printk */
 typedef unsigned long long	llu;
diff --git a/fs/fat/file.c b/fs/fat/file.c
index 531abf9e533c..c27d4e7f698c 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -613,10 +613,8 @@ int fat_setattr(const struct mnt_idmap *idmap, struct dentry *dentry,
 		error = fat_block_truncate_page(inode, attr->ia_size);
 		if (error)
 			goto out;
-		down_write(&MSDOS_I(inode)->truncate_lock);
 		truncate_setsize(inode, attr->ia_size);
 		fat_truncate_blocks(inode, attr->ia_size);
-		up_write(&MSDOS_I(inode)->truncate_lock);
 	}
 
 	/*
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 2a16ed415631..3885ddb916bd 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -216,9 +216,7 @@ static void fat_write_failed(struct address_space *mapping, loff_t to)
 
 	if (to > inode->i_size) {
 		truncate_pagecache(inode, inode->i_size);
-		down_write(&MSDOS_I(inode)->truncate_lock);
 		fat_truncate_blocks(inode, inode->i_size);
-		up_write(&MSDOS_I(inode)->truncate_lock);
 	}
 }
 
@@ -290,43 +288,26 @@ static ssize_t fat_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
 	return ret;
 }
 
-static int fat_get_block_bmap(struct inode *inode, sector_t iblock,
-		struct buffer_head *bh_result, int create)
-{
-	struct super_block *sb = inode->i_sb;
-	unsigned long max_blocks = bh_result->b_size >> inode->i_blkbits;
-	int err;
-	sector_t bmap;
-	unsigned long mapped_blocks;
-
-	BUG_ON(create != 0);
-
-	err = fat_bmap(inode, iblock, &bmap, &mapped_blocks, create, true);
-	if (err)
-		return err;
-
-	if (bmap) {
-		map_bh(bh_result, sb, bmap);
-		max_blocks = min(mapped_blocks, max_blocks);
-	}
-
-	bh_result->b_size = max_blocks << sb->s_blocksize_bits;
-
-	return 0;
-}
-
 static sector_t _fat_bmap(struct address_space *mapping, sector_t block)
 {
 	sector_t blocknr;
 
-	/* fat_get_cluster() assumes the requested blocknr isn't truncated. */
-	down_read(&MSDOS_I(mapping->host)->truncate_lock);
-	blocknr = generic_block_bmap(mapping, block, fat_get_block_bmap);
-	up_read(&MSDOS_I(mapping->host)->truncate_lock);
-
+	inode_lock_shared(mapping->host);
+	blocknr = iomap_bmap(mapping, block, &fat_iomap_bmap_ops);
+	inode_unlock_shared(mapping->host);
 	return blocknr;
 }
 
+static int fat_swap_activate(struct swap_info_struct *sis, struct file *file,
+			     sector_t *span)
+{
+#ifdef CONFIG_SWAP
+	return iomap_swapfile_activate(sis, file, span, &fat_iomap_ops);
+#else
+	return -EIO;
+#endif
+}
+
 /*
  * fat_block_truncate_page() zeroes out a mapping from file offset `from'
  * up to the end of the block which corresponds to `from'.
@@ -350,6 +331,7 @@ static const struct address_space_operations fat_aops = {
 	.direct_IO	= fat_direct_IO,
 	.bmap		= _fat_bmap,
 	.migrate_folio	= buffer_migrate_folio,
+	.swap_activate  = fat_swap_activate,
 };
 
 /*
@@ -790,7 +772,6 @@ static struct inode *fat_alloc_inode(struct super_block *sb)
 	if (!ei)
 		return NULL;
 
-	init_rwsem(&ei->truncate_lock);
 	/* Zeroing to allow iput() even if partial initialized inode. */
 	ei->mmu_private = 0;
 	ei->i_start = 0;
diff --git a/fs/fat/iomap.c b/fs/fat/iomap.c
index 7fb24d0296f7..7977147889bb 100644
--- a/fs/fat/iomap.c
+++ b/fs/fat/iomap.c
@@ -69,3 +69,27 @@ static DEFINE_IOMAP_ITER_NEXT(fat_iomap_next, fat_iomap_begin);
 const struct iomap_ops fat_iomap_ops = {
 	.iomap_next = fat_iomap_next
 };
+
+static int fat_iomap_begin_bmap(struct inode *inode, loff_t offset,
+				loff_t length, unsigned int flags,
+				struct iomap *iomap, struct iomap *srcmap)
+{
+	int err;
+
+	err = __fat_iomap_begin(inode, offset, length, flags, iomap);
+	if (err)
+		return err;
+	/*
+	 * iomap_bmap() only returns blocks in an IOMAP_MAPPED range.
+	 * As the old ->bmap() returned KEEP_SIZE blocks, keep that behavior.
+	 */
+	if (iomap->type == IOMAP_UNWRITTEN)
+		iomap->type = IOMAP_MAPPED;
+	return 0;
+}
+
+static DEFINE_IOMAP_ITER_NEXT(fat_iomap_next_bmap, fat_iomap_begin_bmap);
+
+const struct iomap_ops fat_iomap_bmap_ops = {
+	.iomap_next = fat_iomap_next_bmap
+};
-- 
2.43.0

[RFC PATCH 3/8] fat: add iomap direct I/O support

[Matheus Alves de Almeida]

Replace the deprecated ->direct_IO() path with ->{read,write}_iter() and
iomap_dio_rw(). Add fat_file_open() to advertise direct I/O support.

Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
---
 fs/fat/Kconfig |   1 -
 fs/fat/fat.h   |   1 +
 fs/fat/file.c  | 119 ++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/fat/inode.c |  36 ---------------
 fs/fat/iomap.c |  24 ++++++++++
 5 files changed, 142 insertions(+), 39 deletions(-)

diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
index d31d7866ce19..7989c6bf0fb6 100644
--- a/fs/fat/Kconfig
+++ b/fs/fat/Kconfig
@@ -4,7 +4,6 @@ config FAT_FS
 	select BUFFER_HEAD
 	select FS_IOMAP
 	select NLS
-	select LEGACY_DIRECT_IO
 	help
 	  If you want to use one of the FAT-based file systems (the MS-DOS and
 	  VFAT (Windows 95) file systems), then you must say Y or M here
diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 81c454f8a78a..e14d6d9a6ac1 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -492,6 +492,7 @@ extern const struct export_operations fat_export_ops_nostale;
 /* fat/iomap.c */
 extern const struct iomap_ops fat_iomap_ops;
 extern const struct iomap_ops fat_iomap_bmap_ops;
+extern const struct iomap_dio_ops fat_iomap_write_dio_ops;
 
 /* helper for printk */
 typedef unsigned long long	llu;
diff --git a/fs/fat/file.c b/fs/fat/file.c
index c27d4e7f698c..177a26186ddc 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -201,11 +201,126 @@ int fat_file_fsync(struct file *filp, loff_t start, loff_t end, int datasync)
 	return blkdev_issue_flush(inode->i_sb->s_bdev);
 }
 
+static int fat_file_open(struct inode *inode, struct file *file)
+{
+	int err;
+
+	err = generic_file_open(inode, file);
+	if (err)
+		return err;
+
+	file->f_mode |= FMODE_CAN_ODIRECT;
+	return 0;
+}
+
+static ssize_t fat_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
+{
+	struct inode *inode = file_inode(iocb->ki_filp);
+	ssize_t ret;
+
+	if ((iocb->ki_flags & IOCB_DIRECT) == 0)
+		return generic_file_read_iter(iocb, iter);
+
+	inode_lock_shared(inode);
+	file_accessed(iocb->ki_filp);
+	ret = iomap_dio_rw(iocb, iter, &fat_iomap_ops, NULL, 0, NULL, 0);
+	inode_unlock_shared(inode);
+	return ret;
+}
+
+static ssize_t fat_file_dio_write_iter(struct kiocb *iocb,
+				       struct iov_iter *iter)
+{
+	struct inode *inode = file_inode(iocb->ki_filp);
+	struct msdos_inode_info *mi = MSDOS_I(inode);
+	unsigned int dio_flags = 0;
+	ssize_t ret;
+
+	inode_lock(inode);
+	ret = generic_write_checks(iocb, iter);
+	if (ret <= 0)
+		goto out;
+
+	/* We do not support DIO writes which would need zeroing */
+	if (iocb->ki_pos + iov_iter_count(iter) > mi->mmu_private) {
+		ret = -ENOTBLK;
+		goto out;
+	}
+
+	ret = file_modified(iocb->ki_filp);
+	if (ret)
+		goto out;
+
+	/* Wait under the inode lock for size updates and partial-block zeroing. */
+	if (iocb->ki_pos + iov_iter_count(iter) > i_size_read(inode) ||
+	    !IS_ALIGNED(iocb->ki_pos | iov_iter_alignment(iter),
+			i_blocksize(inode)))
+		dio_flags |= IOMAP_DIO_FORCE_WAIT;
+
+	ret = iomap_dio_rw(iocb, iter, &fat_iomap_ops,
+			   &fat_iomap_write_dio_ops, dio_flags, NULL, 0);
+out:
+	inode_unlock(inode);
+	return ret;
+}
+
+static ssize_t fat_file_write_iter_buffered_fallback(struct kiocb *iocb,
+						     struct iov_iter *iter)
+{
+	ssize_t ret;
+	loff_t end, offset;
+	int err;
+
+	iocb->ki_flags &= ~IOCB_DIRECT;
+	ret = generic_file_write_iter(iocb, iter);
+	if (ret <= 0)
+		return ret;
+
+	offset = iocb->ki_pos - ret;
+	end = iocb->ki_pos - 1;
+	err = filemap_write_and_wait_range(iocb->ki_filp->f_mapping, offset,
+					   end);
+	if (err)
+		return err;
+
+	invalidate_mapping_pages(iocb->ki_filp->f_mapping,
+				 offset >> PAGE_SHIFT,
+				 end >> PAGE_SHIFT);
+
+	return ret;
+}
+
+static ssize_t fat_file_write_iter(struct kiocb *iocb, struct iov_iter *iter)
+{
+	ssize_t ret;
+
+	if ((iocb->ki_flags & IOCB_DIRECT) == 0)
+		return generic_file_write_iter(iocb, iter);
+
+	ret = fat_file_dio_write_iter(iocb, iter);
+	if (ret == -ENOTBLK)
+		ret = 0;
+	else if (ret < 0)
+		return ret;
+
+	/* Fall back to buffered I/O if we could not do DIO */
+	if (iov_iter_count(iter)) {
+		ssize_t done;
+
+		done = fat_file_write_iter_buffered_fallback(iocb, iter);
+		if (done < 0)
+			return done;
+		ret += done;
+	}
+
+	return ret;
+}
 
 const struct file_operations fat_file_operations = {
+	.open		= fat_file_open,
 	.llseek		= generic_file_llseek,
-	.read_iter	= generic_file_read_iter,
-	.write_iter	= generic_file_write_iter,
+	.read_iter	= fat_file_read_iter,
+	.write_iter	= fat_file_write_iter,
 	.mmap_prepare	= generic_file_mmap_prepare,
 	.release	= fat_file_release,
 	.unlocked_ioctl	= fat_generic_ioctl,
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 3885ddb916bd..953706aab68b 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -253,41 +253,6 @@ static int fat_write_end(const struct kiocb *iocb,
 	return err;
 }
 
-static ssize_t fat_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
-{
-	struct file *file = iocb->ki_filp;
-	struct address_space *mapping = file->f_mapping;
-	struct inode *inode = mapping->host;
-	size_t count = iov_iter_count(iter);
-	loff_t offset = iocb->ki_pos;
-	ssize_t ret;
-
-	if (iov_iter_rw(iter) == WRITE) {
-		/*
-		 * FIXME: blockdev_direct_IO() doesn't use ->write_begin(),
-		 * so we need to update the ->mmu_private to block boundary.
-		 *
-		 * But we must fill the remaining area or hole by nul for
-		 * updating ->mmu_private.
-		 *
-		 * Return 0, and fallback to normal buffered write.
-		 */
-		loff_t size = offset + count;
-		if (MSDOS_I(inode)->mmu_private < size)
-			return 0;
-	}
-
-	/*
-	 * FAT need to use the DIO_LOCKING for avoiding the race
-	 * condition of fat_get_block() and ->truncate().
-	 */
-	ret = blockdev_direct_IO(iocb, inode, iter, fat_get_block);
-	if (ret < 0 && iov_iter_rw(iter) == WRITE)
-		fat_write_failed(mapping, offset + count);
-
-	return ret;
-}
-
 static sector_t _fat_bmap(struct address_space *mapping, sector_t block)
 {
 	sector_t blocknr;
@@ -328,7 +293,6 @@ static const struct address_space_operations fat_aops = {
 	.writepages	= fat_writepages,
 	.write_begin	= fat_write_begin,
 	.write_end	= fat_write_end,
-	.direct_IO	= fat_direct_IO,
 	.bmap		= _fat_bmap,
 	.migrate_folio	= buffer_migrate_folio,
 	.swap_activate  = fat_swap_activate,
diff --git a/fs/fat/iomap.c b/fs/fat/iomap.c
index 7977147889bb..8a697deba9d4 100644
--- a/fs/fat/iomap.c
+++ b/fs/fat/iomap.c
@@ -26,6 +26,10 @@ static int __fat_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 		return 0;
 	}
 
+	/* Keep the same behavior of syncing metadata on direct writes */
+	if (flags & IOMAP_WRITE)
+		iomap->flags |= IOMAP_F_DIRTY;
+
 	/*
 	 * We need to separate normal data clusters and
 	 * uninitialized FALLOC_FL_KEEP_SIZE clusters.
@@ -93,3 +97,23 @@ static DEFINE_IOMAP_ITER_NEXT(fat_iomap_next_bmap, fat_iomap_begin_bmap);
 const struct iomap_ops fat_iomap_bmap_ops = {
 	.iomap_next = fat_iomap_next_bmap
 };
+
+static int fat_dio_write_end_io(struct kiocb *iocb, ssize_t size, int error,
+				unsigned int flags)
+{
+	struct inode *inode = file_inode(iocb->ki_filp);
+
+	if (error)
+		return error;
+
+	if (size && i_size_read(inode) < iocb->ki_pos + size) {
+		i_size_write(inode, iocb->ki_pos + size);
+		mark_inode_dirty(inode);
+	}
+
+	return 0;
+}
+
+const struct iomap_dio_ops fat_iomap_write_dio_ops = {
+	.end_io = fat_dio_write_end_io
+};
-- 
2.43.0

Re: [RFC PATCH 3/8] fat: add iomap direct I/O support

[OGAWA Hirofumi]

Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br> writes:

> Replace the deprecated ->direct_IO() path with ->{read,write}_iter() and
> iomap_dio_rw(). Add fat_file_open() to advertise direct I/O support.
>
> Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
> ---
>  fs/fat/Kconfig |   1 -
>  fs/fat/fat.h   |   1 +
>  fs/fat/file.c  | 119 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  fs/fat/inode.c |  36 ---------------
>  fs/fat/iomap.c |  24 ++++++++++
>  5 files changed, 142 insertions(+), 39 deletions(-)
>

[...]

> diff --git a/fs/fat/file.c b/fs/fat/file.c
> index c27d4e7f698c..177a26186ddc 100644
> --- a/fs/fat/file.c
> +++ b/fs/fat/file.c
> @@ -201,11 +201,126 @@ int fat_file_fsync(struct file *filp, loff_t start, loff_t end, int datasync)
>  	return blkdev_issue_flush(inode->i_sb->s_bdev);
>  }
>  

[...]

> +static ssize_t fat_file_write_iter(struct kiocb *iocb, struct iov_iter *iter)
> +{
> +	ssize_t ret;
> +
> +	if ((iocb->ki_flags & IOCB_DIRECT) == 0)
> +		return generic_file_write_iter(iocb, iter);
> +
> +	ret = fat_file_dio_write_iter(iocb, iter);
> +	if (ret == -ENOTBLK)
> +		ret = 0;
> +	else if (ret < 0)
> +		return ret;
> +
> +	/* Fall back to buffered I/O if we could not do DIO */
> +	if (iov_iter_count(iter)) {
> +		ssize_t done;
> +
> +		done = fat_file_write_iter_buffered_fallback(iocb, iter);
> +		if (done < 0)
> +			return done;

If fat_file_dio_write_iter() wrote some, this should return ret (how
many written), instead of the error? Or it never happen?

> +		ret += done;
> +	}
> +
> +	return ret;
> +}

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[RFC PATCH 4/8] fat: clean stale bdev buffers of newly allocated clusters

[Matheus Alves de Almeida]

Directory I/O uses buffer_heads on the block device, which may leave
stale buffers behind when clusters are freed and reused. The buffer_head
I/O paths currently call clean_bdev_aliases() for newly allocated blocks,
preventing stale buffers from overwriting new data.

Buffered iomap I/O does not provide this protection. In preparation for
the buffered iomap conversion, clean bdev aliases on newly allocated
clusters, like exfat patch "exfat: clean stale bdev buffers of newly
allocated clusters".

Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
---
 fs/fat/fatent.c | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
index 4fa1fae5abf9..46899efa3cf8 100644
--- a/fs/fat/fatent.c
+++ b/fs/fat/fatent.c
@@ -523,6 +523,21 @@ static int fat_flush_bhs(struct super_block *sb, struct buffer_head **bhs,
 	return 0;
 }
 
+/*
+ * Newly allocated clusters may still have dirty buffer_heads in the block
+ * device mapping, e.g. the dentries of a directory that was just removed.
+ * File data does not go through those buffers, so drop them; otherwise a
+ * later flush of the block device writes them over the new data.
+ */
+static void fat_clean_bdev_aliases(struct super_block *sb,
+				   unsigned int clu, unsigned int count)
+{
+	struct msdos_sb_info *sbi = MSDOS_SB(sb);
+
+	clean_bdev_aliases(sb->s_bdev, fat_clus_to_blknr(sbi, clu),
+			   (sector_t)count * sbi->sec_per_clus);
+}
+
 static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluster,
 				bool return_all)
 {
@@ -531,7 +546,7 @@ static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluste
 	const struct fatent_operations *ops = sbi->fatent_ops;
 	struct fat_entry fatent, prev_ent;
 	struct buffer_head *bhs[MAX_BUF_PER_PAGE];
-	int i, count, err, nr_bhs, idx_clus;
+	int i, count, err, nr_bhs, idx_clus, cont_start, cont_len;
 
 	lock_fat(sbi);
 	if (sbi->free_clusters != -1 && sbi->free_clus_valid &&
@@ -540,14 +555,20 @@ static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluste
 		return -ENOSPC;
 	}
 
+	cont_len = 0;
 	err = nr_bhs = idx_clus = 0;
 	count = FAT_START_ENT;
 	fatent_init(&prev_ent);
 	fatent_init(&fatent);
 	fatent_set_entry(&fatent, sbi->prev_free + 1);
 	while (count < sbi->max_cluster) {
-		if (fatent.entry >= sbi->max_cluster)
+		if (fatent.entry >= sbi->max_cluster) {
+			if (cont_len) {
+				fat_clean_bdev_aliases(sb, cont_start, cont_len);
+				cont_len = 0;
+			}
 			fatent.entry = FAT_START_ENT;
+		}
 		fatent_set_entry(&fatent, fatent.entry);
 		err = fat_ent_read_block(sb, &fatent);
 		if (err)
@@ -573,6 +594,10 @@ static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluste
 					*cluster = entry;
 				idx_clus++;
 
+				if (cont_len == 0)
+					cont_start = entry;
+				cont_len++;
+
 				if (nr_bhs + fatent.nr_bhs > MAX_BUF_PER_PAGE) {
 					err = fat_flush_bhs(sb, bhs, &nr_bhs,
 							    inode_needs_sync(inode));
@@ -589,6 +614,9 @@ static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluste
 				 * so we can still use the prev_ent.
 				 */
 				prev_ent = fatent;
+			} else if (cont_len) {
+				fat_clean_bdev_aliases(sb, cont_start, cont_len);
+				cont_len = 0;
 			}
 			count++;
 			if (count == sbi->max_cluster)
@@ -602,6 +630,8 @@ static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluste
 	err = -ENOSPC;
 
 out:
+	if (cont_len)
+		fat_clean_bdev_aliases(sb, cont_start, cont_len);
 	unlock_fat(sbi);
 	mark_fsinfo_dirty(sb);
 	fatent_brelse(&fatent);
-- 
2.43.0

Re: [RFC PATCH 4/8] fat: clean stale bdev buffers of newly allocated clusters

[OGAWA Hirofumi]

Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br> writes:

> Directory I/O uses buffer_heads on the block device, which may leave
> stale buffers behind when clusters are freed and reused. The buffer_head
> I/O paths currently call clean_bdev_aliases() for newly allocated blocks,
> preventing stale buffers from overwriting new data.
>
> Buffered iomap I/O does not provide this protection. In preparation for
> the buffered iomap conversion, clean bdev aliases on newly allocated
> clusters, like exfat patch "exfat: clean stale bdev buffers of newly
> allocated clusters".
>
> Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
> ---
>  fs/fat/fatent.c | 34 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
> index 4fa1fae5abf9..46899efa3cf8 100644
> --- a/fs/fat/fatent.c
> +++ b/fs/fat/fatent.c
> @@ -523,6 +523,21 @@ static int fat_flush_bhs(struct super_block *sb, struct buffer_head **bhs,
>  	return 0;
>  }
>  
> +/*
> + * Newly allocated clusters may still have dirty buffer_heads in the block
> + * device mapping, e.g. the dentries of a directory that was just removed.
> + * File data does not go through those buffers, so drop them; otherwise a
> + * later flush of the block device writes them over the new data.
> + */
> +static void fat_clean_bdev_aliases(struct super_block *sb,
> +				   unsigned int clu, unsigned int count)
> +{
> +	struct msdos_sb_info *sbi = MSDOS_SB(sb);
> +
> +	clean_bdev_aliases(sb->s_bdev, fat_clus_to_blknr(sbi, clu),
> +			   (sector_t)count * sbi->sec_per_clus);
> +}
> +
>  static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluster,
>  				bool return_all)
>  {
> @@ -531,7 +546,7 @@ static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluste
>  	const struct fatent_operations *ops = sbi->fatent_ops;
>  	struct fat_entry fatent, prev_ent;
>  	struct buffer_head *bhs[MAX_BUF_PER_PAGE];
> -	int i, count, err, nr_bhs, idx_clus;
> +	int i, count, err, nr_bhs, idx_clus, cont_start, cont_len;
>  
>  	lock_fat(sbi);
>  	if (sbi->free_clusters != -1 && sbi->free_clus_valid &&
> @@ -540,14 +555,20 @@ static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluste
>  		return -ENOSPC;
>  	}
>  
> +	cont_len = 0;
>  	err = nr_bhs = idx_clus = 0;
>  	count = FAT_START_ENT;
>  	fatent_init(&prev_ent);
>  	fatent_init(&fatent);
>  	fatent_set_entry(&fatent, sbi->prev_free + 1);
>  	while (count < sbi->max_cluster) {
> -		if (fatent.entry >= sbi->max_cluster)
> +		if (fatent.entry >= sbi->max_cluster) {
> +			if (cont_len) {
> +				fat_clean_bdev_aliases(sb, cont_start, cont_len);
> +				cont_len = 0;
> +			}
>  			fatent.entry = FAT_START_ENT;
> +		}
>  		fatent_set_entry(&fatent, fatent.entry);
>  		err = fat_ent_read_block(sb, &fatent);
>  		if (err)
> @@ -573,6 +594,10 @@ static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluste
>  					*cluster = entry;
>  				idx_clus++;
>  
> +				if (cont_len == 0)
> +					cont_start = entry;
> +				cont_len++;
> +
>  				if (nr_bhs + fatent.nr_bhs > MAX_BUF_PER_PAGE) {
>  					err = fat_flush_bhs(sb, bhs, &nr_bhs,
>  							    inode_needs_sync(inode));
> @@ -589,6 +614,9 @@ static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluste
>  				 * so we can still use the prev_ent.
>  				 */
>  				prev_ent = fatent;
> +			} else if (cont_len) {
> +				fat_clean_bdev_aliases(sb, cont_start, cont_len);
> +				cont_len = 0;
>  			}
>  			count++;
>  			if (count == sbi->max_cluster)
> @@ -602,6 +630,8 @@ static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluste
>  	err = -ENOSPC;
>  
>  out:
> +	if (cont_len)
> +		fat_clean_bdev_aliases(sb, cont_start, cont_len);

We would want to make as possible as shorter lock_fat. At least, this
part can move outside of unlock_fat(sbi)?

>  	unlock_fat(sbi);
>  	mark_fsinfo_dirty(sb);
>  	fatent_brelse(&fatent);

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[RFC PATCH 5/8] fat: drop ->write_begin() and ->write_end() usage from fat_cont_expand()

[Matheus Alves de Almeida]

fat_cont_expand() currently relies on ->write_begin() and ->write_end()
to zero the new file range. These callbacks will no longer be available
after the iomap conversion, so a new method for zeroing is needed.

Zero new ranges folio-by-folio instead.

Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
---
 fs/fat/file.c | 122 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 76 insertions(+), 46 deletions(-)

diff --git a/fs/fat/file.c b/fs/fat/file.c
index 177a26186ddc..8689cbbdad13 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -18,6 +18,8 @@
 #include <linux/security.h>
 #include <linux/falloc.h>
 #include <linux/fileattr.h>
+#include <linux/rmap.h>
+#include <linux/writeback.h>
 #include "fat.h"
 
 static long fat_fallocate(struct file *file, int mode,
@@ -353,68 +355,96 @@ static int fat_preallocate(struct inode *inode, loff_t new_size)
 	return fat_add_clusters(inode, nr_cluster);
 }
 
-static int fat_cont_expand_simple(struct inode *inode, loff_t size)
+static int fat_zero_and_extend(struct inode *inode, loff_t new_size)
 {
+	struct msdos_inode_info *mi = MSDOS_I(inode);
 	struct address_space *mapping = inode->i_mapping;
-	const struct address_space_operations *aops = mapping->a_ops;
+	loff_t pos = i_size_read(inode), fpos, next;
+	loff_t old_size = pos;
 	struct folio *folio;
-	void *fsdata = NULL;
-	int err;
+	size_t from, to;
+	pgoff_t index;
 
-	err = aops->write_begin(NULL, mapping, size, 0, &folio, &fsdata);
-	if (err)
-		goto out;
+	while (pos < new_size) {
+		index = pos >> PAGE_SHIFT;
 
-	err = aops->write_end(NULL, mapping, size, 0, 0, folio, fsdata);
+		folio = read_mapping_folio(mapping, index, NULL);
+		if (IS_ERR(folio))
+			return PTR_ERR(folio);
 
-out:
-	return err;
+		folio_lock(folio);
+		if (folio->mapping != mapping) {
+			folio_unlock(folio);
+			folio_put(folio);
+			continue;
+		}
+		folio_wait_stable(folio);
+
+		fpos = folio_pos(folio);
+		next = min_t(loff_t, new_size, fpos + folio_size(folio));
+		from = pos - fpos;
+		to = next - fpos;
+
+		/*
+		 * An existing shared writeable mmap of the file could allow
+		 * writing into the newly zeroed range, so make it read-only.
+		 */
+		if (pos == old_size && from)
+			folio_mkclean(folio);
+
+		folio_zero_segment(folio, from, to);
+		folio_mark_dirty(folio);
+		mi->mmu_private = max(mi->mmu_private, next);
+		i_size_write(inode, next);
+
+		folio_unlock(folio);
+		folio_put(folio);
+
+		balance_dirty_pages_ratelimited(mapping);
+		if (fatal_signal_pending(current))
+			return -EINTR;
+
+		pos = next;
+	}
+
+	return 0;
 }
 
-static int fat_cont_expand(struct inode *inode, loff_t size)
+static int fat_cont_expand(struct inode *inode, loff_t new_size)
 {
-	struct address_space *mapping = inode->i_mapping;
-	loff_t start = inode->i_size, count = size - inode->i_size;
+	struct msdos_inode_info *mi = MSDOS_I(inode);
+	loff_t old_alloc = inode_get_bytes(inode);
+	loff_t old_size = i_size_read(inode);
+	loff_t old_mmu_private = mi->mmu_private;
 	int err;
 
-	/*
-	 * Preallocating space here is more efficient than during
-	 * fat_cont_expand_simple() because of batching.
-	 */
-	err = inode_newsize_ok(inode, size);
-	if (err)
-		goto out;
-	err = fat_preallocate(inode, size);
-	if (err)
-		goto out;
-	err = fat_cont_expand_simple(inode, size);
+	err = inode_newsize_ok(inode, new_size);
 	if (err)
-		goto out;
+		return err;
+	inode_dio_wait(inode);
+	err = fat_preallocate(inode, new_size);
+	if (err) {
+		/* fat_chain_add() does not rollback on failure */
+		fat_truncate_blocks(inode, old_alloc);
+		return err;
+	}
+	err = fat_zero_and_extend(inode, new_size);
+	if (err) {
+		filemap_invalidate_lock(inode->i_mapping);
+		truncate_setsize(inode, old_size);
+		fat_truncate_blocks(inode, old_alloc);
+		mi->mmu_private = old_mmu_private;
+		filemap_invalidate_unlock(inode->i_mapping);
+		return err;
+	}
 
 	fat_truncate_time(inode, NULL, FAT_UPDATE_CMTIME);
+	mi->i_attrs |= ATTR_ARCH;
 	mark_inode_dirty(inode);
-	if (IS_SYNC(inode)) {
-		int err2;
+	if (IS_SYNC(inode))
+		return write_inode_now(inode, 1);
 
-		/*
-		 * Opencode syncing since we don't have a file open to use
-		 * standard fsync path.
-		 */
-		err = filemap_fdatawrite_range(mapping, start,
-					       start + count - 1);
-		err2 = mmb_sync(&MSDOS_I(inode)->i_metadata_bhs);
-		if (!err)
-			err = err2;
-		err2 = write_inode_now(inode, 1);
-		if (!err)
-			err = err2;
-		if (!err) {
-			err =  filemap_fdatawait_range(mapping, start,
-						       start + count - 1);
-		}
-	}
-out:
-	return err;
+	return 0;
 }
 
 /*
-- 
2.43.0

Re: [RFC PATCH 5/8] fat: drop ->write_begin() and ->write_end() usage from fat_cont_expand()

[OGAWA Hirofumi]

Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br> writes:

> fat_cont_expand() currently relies on ->write_begin() and ->write_end()
> to zero the new file range. These callbacks will no longer be available
> after the iomap conversion, so a new method for zeroing is needed.
>
> Zero new ranges folio-by-folio instead.
>
> Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
> ---
>  fs/fat/file.c | 122 +++++++++++++++++++++++++++++++-------------------
>  1 file changed, 76 insertions(+), 46 deletions(-)
>
> diff --git a/fs/fat/file.c b/fs/fat/file.c
> index 177a26186ddc..8689cbbdad13 100644
> --- a/fs/fat/file.c
> +++ b/fs/fat/file.c
> @@ -18,6 +18,8 @@
>  #include <linux/security.h>
>  #include <linux/falloc.h>
>  #include <linux/fileattr.h>
> +#include <linux/rmap.h>
> +#include <linux/writeback.h>
>  #include "fat.h"
>  
>  static long fat_fallocate(struct file *file, int mode,
> @@ -353,68 +355,96 @@ static int fat_preallocate(struct inode *inode, loff_t new_size)
>  	return fat_add_clusters(inode, nr_cluster);
>  }
>  
> -static int fat_cont_expand_simple(struct inode *inode, loff_t size)
> +static int fat_zero_and_extend(struct inode *inode, loff_t new_size)
>  {
> +	struct msdos_inode_info *mi = MSDOS_I(inode);
>  	struct address_space *mapping = inode->i_mapping;
> -	const struct address_space_operations *aops = mapping->a_ops;
> +	loff_t pos = i_size_read(inode), fpos, next;
> +	loff_t old_size = pos;
>  	struct folio *folio;
> -	void *fsdata = NULL;
> -	int err;
> +	size_t from, to;
> +	pgoff_t index;
>  
> -	err = aops->write_begin(NULL, mapping, size, 0, &folio, &fsdata);
> -	if (err)
> -		goto out;
> +	while (pos < new_size) {
> +		index = pos >> PAGE_SHIFT;
>  
> -	err = aops->write_end(NULL, mapping, size, 0, 0, folio, fsdata);
> +		folio = read_mapping_folio(mapping, index, NULL);

Hm, is this reading blocks from the device for zeroing blocks even if
page is completely unwritten block? If so, we would not want to read
from the device.

> +		if (IS_ERR(folio))
> +			return PTR_ERR(folio);
>  
> -out:
> -	return err;
> +		folio_lock(folio);
> +		if (folio->mapping != mapping) {
> +			folio_unlock(folio);
> +			folio_put(folio);
> +			continue;
> +		}
> +		folio_wait_stable(folio);

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[RFC PATCH 6/8] fat: add iomap buffered I/O support

[Matheus Alves de Almeida]

Use iomap for buffered I/O, replacing buffer_heads. Remove
fat_get_block(), fat_write_begin(), fat_write_failed() as the iomap path
makes them unnecessary.

Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
---
 fs/fat/fat.h   |   4 +-
 fs/fat/file.c  |  72 +++++++++++++++++++----
 fs/fat/inode.c | 157 +++++--------------------------------------------
 fs/fat/iomap.c |  84 ++++++++++++++++++++++++++
 4 files changed, 164 insertions(+), 153 deletions(-)

diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index e14d6d9a6ac1..78c6fb5cdc46 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -415,9 +415,9 @@ extern int fat_getattr(const struct mnt_idmap *idmap,
 int fat_fileattr_get(struct dentry *dentry, struct file_kattr *fa);
 extern int fat_file_fsync(struct file *file, loff_t start, loff_t end,
 			  int datasync);
+int fat_preallocate(struct inode *inode, loff_t new_size);
 
 /* fat/inode.c */
-extern int fat_block_truncate_page(struct inode *inode, loff_t from);
 extern void fat_attach(struct inode *inode, loff_t i_pos);
 extern void fat_detach(struct inode *inode);
 extern struct inode *fat_iget(struct super_block *sb, loff_t i_pos);
@@ -492,7 +492,9 @@ extern const struct export_operations fat_export_ops_nostale;
 /* fat/iomap.c */
 extern const struct iomap_ops fat_iomap_ops;
 extern const struct iomap_ops fat_iomap_bmap_ops;
+extern const struct iomap_ops fat_iomap_write_ops;
 extern const struct iomap_dio_ops fat_iomap_write_dio_ops;
+extern const struct iomap_writeback_ops fat_writeback_ops;
 
 /* helper for printk */
 typedef unsigned long long	llu;
diff --git a/fs/fat/file.c b/fs/fat/file.c
index 8689cbbdad13..723b671d5b4b 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -220,12 +220,13 @@ static ssize_t fat_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
 	struct inode *inode = file_inode(iocb->ki_filp);
 	ssize_t ret;
 
-	if ((iocb->ki_flags & IOCB_DIRECT) == 0)
-		return generic_file_read_iter(iocb, iter);
-
 	inode_lock_shared(inode);
-	file_accessed(iocb->ki_filp);
-	ret = iomap_dio_rw(iocb, iter, &fat_iomap_ops, NULL, 0, NULL, 0);
+	if (iocb->ki_flags & IOCB_DIRECT) {
+		file_accessed(iocb->ki_filp);
+		ret = iomap_dio_rw(iocb, iter, &fat_iomap_ops, NULL, 0, NULL, 0);
+	} else {
+		ret = generic_file_read_iter(iocb, iter);
+	}
 	inode_unlock_shared(inode);
 	return ret;
 }
@@ -266,6 +267,50 @@ static ssize_t fat_file_dio_write_iter(struct kiocb *iocb,
 	return ret;
 }
 
+static int fat_cont_expand(struct inode *inode, loff_t new_size);
+
+static ssize_t fat_file_write_iter_buffered(struct kiocb *iocb,
+					    struct iov_iter *iter)
+{
+	struct inode *inode = file_inode(iocb->ki_filp);
+	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
+	loff_t offset, old_size, old_alloc, keep_alloc;
+	ssize_t ret;
+
+	inode_lock(inode);
+	ret = generic_write_checks(iocb, iter);
+	if (ret <= 0)
+		goto out;
+
+	ret = file_modified(iocb->ki_filp);
+	if (ret)
+		goto out;
+
+	offset = iocb->ki_pos;
+	old_size = i_size_read(inode);
+	if (offset > old_size) {
+		/* We need to fill this hole */
+		ret = fat_cont_expand(inode, offset);
+		if (ret)
+			goto out;
+	}
+
+	inode_dio_wait(inode);
+	old_alloc = inode_get_bytes(inode);
+	ret = iomap_file_buffered_write(iocb, iter, &fat_iomap_write_ops,
+					NULL, NULL);
+	keep_alloc = max(old_alloc, round_up(i_size_read(inode),
+					     sbi->cluster_size));
+	if (inode_get_bytes(inode) > keep_alloc)
+		fat_truncate_blocks(inode, keep_alloc);
+	if (ret > 0)
+		ret = generic_write_sync(iocb, ret);
+
+out:
+	inode_unlock(inode);
+	return ret;
+}
+
 static ssize_t fat_file_write_iter_buffered_fallback(struct kiocb *iocb,
 						     struct iov_iter *iter)
 {
@@ -274,7 +319,7 @@ static ssize_t fat_file_write_iter_buffered_fallback(struct kiocb *iocb,
 	int err;
 
 	iocb->ki_flags &= ~IOCB_DIRECT;
-	ret = generic_file_write_iter(iocb, iter);
+	ret = fat_file_write_iter_buffered(iocb, iter);
 	if (ret <= 0)
 		return ret;
 
@@ -297,7 +342,7 @@ static ssize_t fat_file_write_iter(struct kiocb *iocb, struct iov_iter *iter)
 	ssize_t ret;
 
 	if ((iocb->ki_flags & IOCB_DIRECT) == 0)
-		return generic_file_write_iter(iocb, iter);
+		return fat_file_write_iter_buffered(iocb, iter);
 
 	ret = fat_file_dio_write_iter(iocb, iter);
 	if (ret == -ENOTBLK)
@@ -334,7 +379,7 @@ const struct file_operations fat_file_operations = {
 	.setlease	= generic_setlease,
 };
 
-static int fat_preallocate(struct inode *inode, loff_t new_size)
+int fat_preallocate(struct inode *inode, loff_t new_size)
 {
 	int nr_cluster; /* Number of clusters to be allocated */
 	loff_t mm_bytes; /* Number of bytes to be allocated for file */
@@ -755,11 +800,16 @@ int fat_setattr(const struct mnt_idmap *idmap, struct dentry *dentry,
 	}
 
 	if (attr->ia_valid & ATTR_SIZE) {
-		error = fat_block_truncate_page(inode, attr->ia_size);
+		filemap_invalidate_lock(inode->i_mapping);
+		error = iomap_truncate_page(inode, attr->ia_size, NULL,
+					    &fat_iomap_ops, NULL, NULL);
+		if (!error) {
+			truncate_setsize(inode, attr->ia_size);
+			fat_truncate_blocks(inode, attr->ia_size);
+		}
+		filemap_invalidate_unlock(inode->i_mapping);
 		if (error)
 			goto out;
-		truncate_setsize(inode, attr->ia_size);
-		fat_truncate_blocks(inode, attr->ia_size);
 	}
 
 	/*
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 953706aab68b..9df10e5a1ede 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -116,141 +116,27 @@ int fat_add_clusters(struct inode *inode, int nr_cluster)
 	return err;
 }
 
-static inline int __fat_get_block(struct inode *inode, sector_t iblock,
-				  unsigned long *max_blocks,
-				  struct buffer_head *bh_result, int create)
-{
-	struct super_block *sb = inode->i_sb;
-	struct msdos_sb_info *sbi = MSDOS_SB(sb);
-	unsigned long mapped_blocks;
-	sector_t phys, last_block;
-	int err, offset;
-
-	err = fat_bmap(inode, iblock, &phys, &mapped_blocks, create, false);
-	if (err)
-		return err;
-	if (phys) {
-		map_bh(bh_result, sb, phys);
-		*max_blocks = min(mapped_blocks, *max_blocks);
-		return 0;
-	}
-	if (!create)
-		return 0;
-
-	if (iblock != MSDOS_I(inode)->mmu_private >> sb->s_blocksize_bits) {
-		fat_fs_error(sb, "corrupted file size (i_pos %lld, %lld)",
-			MSDOS_I(inode)->i_pos, MSDOS_I(inode)->mmu_private);
-		return -EIO;
-	}
-
-	last_block = inode->i_blocks >> (sb->s_blocksize_bits - 9);
-	offset = (unsigned long)iblock & (sbi->sec_per_clus - 1);
-	/*
-	 * allocate a cluster according to the following.
-	 * 1) no more available blocks
-	 * 2) not part of fallocate region
-	 */
-	if (!offset && !(iblock < last_block)) {
-		/* TODO: multiple cluster allocation would be desirable. */
-		err = fat_add_clusters(inode, 1);
-		if (err)
-			return err;
-	}
-	/* available blocks on this cluster */
-	mapped_blocks = sbi->sec_per_clus - offset;
-
-	*max_blocks = min(mapped_blocks, *max_blocks);
-	MSDOS_I(inode)->mmu_private += *max_blocks << sb->s_blocksize_bits;
-
-	err = fat_bmap(inode, iblock, &phys, &mapped_blocks, create, false);
-	if (err)
-		return err;
-	if (!phys) {
-		fat_fs_error(sb,
-			     "invalid FAT chain (i_pos %lld, last_block %llu)",
-			     MSDOS_I(inode)->i_pos,
-			     (unsigned long long)last_block);
-		return -EIO;
-	}
-
-	BUG_ON(*max_blocks != mapped_blocks);
-	set_buffer_new(bh_result);
-	map_bh(bh_result, sb, phys);
-
-	return 0;
-}
-
-static int fat_get_block(struct inode *inode, sector_t iblock,
-			 struct buffer_head *bh_result, int create)
-{
-	struct super_block *sb = inode->i_sb;
-	unsigned long max_blocks = bh_result->b_size >> inode->i_blkbits;
-	int err;
-
-	err = __fat_get_block(inode, iblock, &max_blocks, bh_result, create);
-	if (err)
-		return err;
-	bh_result->b_size = max_blocks << sb->s_blocksize_bits;
-	return 0;
-}
-
 static int fat_writepages(struct address_space *mapping,
 			  struct writeback_control *wbc)
 {
-	return mpage_writepages(mapping, wbc, fat_get_block);
+	struct iomap_writepage_ctx wpc = {
+		.inode = mapping->host,
+		.wbc = wbc,
+		.ops = &fat_writeback_ops,
+	};
+
+	return iomap_writepages(&wpc);
 }
 
 static int fat_read_folio(struct file *file, struct folio *folio)
 {
-	return mpage_read_folio(folio, fat_get_block);
+	iomap_bio_read_folio(folio, &fat_iomap_ops);
+	return 0;
 }
 
 static void fat_readahead(struct readahead_control *rac)
 {
-	mpage_readahead(rac, fat_get_block);
-}
-
-static void fat_write_failed(struct address_space *mapping, loff_t to)
-{
-	struct inode *inode = mapping->host;
-
-	if (to > inode->i_size) {
-		truncate_pagecache(inode, inode->i_size);
-		fat_truncate_blocks(inode, inode->i_size);
-	}
-}
-
-static int fat_write_begin(const struct kiocb *iocb,
-			   struct address_space *mapping,
-			   loff_t pos, unsigned len,
-			   struct folio **foliop, void **fsdata)
-{
-	int err;
-
-	err = cont_write_begin(iocb, mapping, pos, len,
-				foliop, fsdata, fat_get_block,
-				&MSDOS_I(mapping->host)->mmu_private);
-	if (err < 0)
-		fat_write_failed(mapping, pos + len);
-	return err;
-}
-
-static int fat_write_end(const struct kiocb *iocb,
-			 struct address_space *mapping,
-			 loff_t pos, unsigned len, unsigned copied,
-			 struct folio *folio, void *fsdata)
-{
-	struct inode *inode = mapping->host;
-	int err;
-	err = generic_write_end(iocb, mapping, pos, len, copied, folio, fsdata);
-	if (err < len)
-		fat_write_failed(mapping, pos + len);
-	if (!(err < 0) && !(MSDOS_I(inode)->i_attrs & ATTR_ARCH)) {
-		fat_truncate_time(inode, NULL, FAT_UPDATE_CMTIME);
-		MSDOS_I(inode)->i_attrs |= ATTR_ARCH;
-		mark_inode_dirty(inode);
-	}
-	return err;
+	iomap_bio_readahead(rac, &fat_iomap_ops);
 }
 
 static sector_t _fat_bmap(struct address_space *mapping, sector_t block)
@@ -273,29 +159,18 @@ static int fat_swap_activate(struct swap_info_struct *sis, struct file *file,
 #endif
 }
 
-/*
- * fat_block_truncate_page() zeroes out a mapping from file offset `from'
- * up to the end of the block which corresponds to `from'.
- * This is required during truncate to physically zeroout the tail end
- * of that block so it doesn't yield old data if the file is later grown.
- * Also, avoid causing failure from fsx for cases of "data past EOF"
- */
-int fat_block_truncate_page(struct inode *inode, loff_t from)
-{
-	return block_truncate_page(inode->i_mapping, from, fat_get_block);
-}
-
 static const struct address_space_operations fat_aops = {
-	.dirty_folio	= block_dirty_folio,
-	.invalidate_folio = block_invalidate_folio,
+	.dirty_folio	= iomap_dirty_folio,
+	.invalidate_folio = iomap_invalidate_folio,
 	.read_folio	= fat_read_folio,
 	.readahead	= fat_readahead,
 	.writepages	= fat_writepages,
-	.write_begin	= fat_write_begin,
-	.write_end	= fat_write_end,
 	.bmap		= _fat_bmap,
-	.migrate_folio	= buffer_migrate_folio,
+	.migrate_folio	= filemap_migrate_folio,
 	.swap_activate  = fat_swap_activate,
+	.is_partially_uptodate = iomap_is_partially_uptodate,
+	.error_remove_folio = generic_error_remove_folio,
+	.release_folio = iomap_release_folio,
 };
 
 /*
diff --git a/fs/fat/iomap.c b/fs/fat/iomap.c
index 8a697deba9d4..cfeaea5d352e 100644
--- a/fs/fat/iomap.c
+++ b/fs/fat/iomap.c
@@ -5,6 +5,9 @@
  * Copyright (C) 2026 Matheus Alves de Almeida
  */
 
+#include <linux/highmem.h>
+#include <linux/pagemap.h>
+
 #include "fat.h"
 
 static int __fat_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
@@ -74,6 +77,65 @@ const struct iomap_ops fat_iomap_ops = {
 	.iomap_next = fat_iomap_next
 };
 
+static int fat_iomap_end_write(struct inode *inode, loff_t offset,
+			       loff_t length, ssize_t written,
+			       unsigned int flags, struct iomap *iomap)
+{
+	struct msdos_inode_info *mi = MSDOS_I(inode);
+
+	if (written > 0) {
+		mi->mmu_private = max(mi->mmu_private, offset + written);
+		if (!(mi->i_attrs & ATTR_ARCH)) {
+			mi->i_attrs |= ATTR_ARCH;
+			mark_inode_dirty(inode);
+		}
+	}
+
+	if (iomap->flags & IOMAP_F_SIZE_CHANGED)
+		mark_inode_dirty(inode);
+
+	return 0;
+}
+
+static int fat_iomap_begin_write(struct inode *inode, loff_t offset,
+				 loff_t length, unsigned int flags,
+				 struct iomap *iomap, struct iomap *srcmap)
+{
+	loff_t curr_size = i_size_read(inode);
+	loff_t old_alloc = inode_get_bytes(inode);
+	int err;
+
+	/* Only support extending writes that start at EOF */
+	if (WARN_ONCE(offset > curr_size, "Write starting past EOF"))
+		return -EOPNOTSUPP;
+
+	if (offset == curr_size) {
+		/*
+		 * TODO: allocate multiple blocks while allowing
+		 * short writes
+		 */
+		err = fat_preallocate(inode, offset + 1);
+		if (err)
+			goto err_truncate;
+	}
+
+	err = __fat_iomap_begin(inode, offset, length, flags, iomap);
+	if (err)
+		goto err_truncate;
+
+	return 0;
+err_truncate:
+	fat_truncate_blocks(inode, old_alloc);
+	return err;
+}
+
+static DEFINE_IOMAP_ITER_NEXT_END(fat_iomap_next_write, fat_iomap_begin_write,
+				  fat_iomap_end_write);
+
+const struct iomap_ops fat_iomap_write_ops = {
+	.iomap_next = fat_iomap_next_write
+};
+
 static int fat_iomap_begin_bmap(struct inode *inode, loff_t offset,
 				loff_t length, unsigned int flags,
 				struct iomap *iomap, struct iomap *srcmap)
@@ -117,3 +179,25 @@ static int fat_dio_write_end_io(struct kiocb *iocb, ssize_t size, int error,
 const struct iomap_dio_ops fat_iomap_write_dio_ops = {
 	.end_io = fat_dio_write_end_io
 };
+
+static ssize_t fat_writeback_range(struct iomap_writepage_ctx *wpc,
+				   struct folio *folio, u64 offset,
+				   unsigned int len, u64 end_pos)
+{
+	if (offset < wpc->iomap.offset ||
+	    offset >= wpc->iomap.offset + wpc->iomap.length) {
+		int err;
+
+		err = __fat_iomap_begin(wpc->inode, offset, len, 0,
+					&wpc->iomap);
+		if (err)
+			return err;
+	}
+
+	return iomap_add_to_ioend(wpc, folio, offset, end_pos, len);
+}
+
+const struct iomap_writeback_ops fat_writeback_ops = {
+	.writeback_range = fat_writeback_range,
+	.writeback_submit = iomap_ioend_writeback_submit,
+};
-- 
2.43.0

Re: [RFC PATCH 6/8] fat: add iomap buffered I/O support

[OGAWA Hirofumi]

Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br> writes:

> Use iomap for buffered I/O, replacing buffer_heads. Remove
> fat_get_block(), fat_write_begin(), fat_write_failed() as the iomap path
> makes them unnecessary.
>
> Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
> ---
>  fs/fat/fat.h   |   4 +-
>  fs/fat/file.c  |  72 +++++++++++++++++++----
>  fs/fat/inode.c | 157 +++++--------------------------------------------
>  fs/fat/iomap.c |  84 ++++++++++++++++++++++++++
>  4 files changed, 164 insertions(+), 153 deletions(-)
>

[...]

> diff --git a/fs/fat/file.c b/fs/fat/file.c
> index 8689cbbdad13..723b671d5b4b 100644
> --- a/fs/fat/file.c
> +++ b/fs/fat/file.c

[...]

> +static int fat_cont_expand(struct inode *inode, loff_t new_size);
> +
> +static ssize_t fat_file_write_iter_buffered(struct kiocb *iocb,
> +					    struct iov_iter *iter)
> +{
> +	struct inode *inode = file_inode(iocb->ki_filp);
> +	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
> +	loff_t offset, old_size, old_alloc, keep_alloc;
> +	ssize_t ret;
> +
> +	inode_lock(inode);
> +	ret = generic_write_checks(iocb, iter);
> +	if (ret <= 0)
> +		goto out;
> +
> +	ret = file_modified(iocb->ki_filp);
> +	if (ret)
> +		goto out;
> +
> +	offset = iocb->ki_pos;
> +	old_size = i_size_read(inode);
> +	if (offset > old_size) {
> +		/* We need to fill this hole */
> +		ret = fat_cont_expand(inode, offset);
> +		if (ret)
> +			goto out;
> +	}
> +
> +	inode_dio_wait(inode);
> +	old_alloc = inode_get_bytes(inode);
> +	ret = iomap_file_buffered_write(iocb, iter, &fat_iomap_write_ops,
> +					NULL, NULL);
> +	keep_alloc = max(old_alloc, round_up(i_size_read(inode),
> +					     sbi->cluster_size));
> +	if (inode_get_bytes(inode) > keep_alloc)
> +		fat_truncate_blocks(inode, keep_alloc);

What is doing by this fat_truncate_blocks?

> +	if (ret > 0)
> +		ret = generic_write_sync(iocb, ret);

Maybe we want to do this outside inode_unlock()?

> +out:
> +	inode_unlock(inode);
> +	return ret;
> +}
> +

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[RFC PATCH 7/8] fat: remove ->mmu_private

[Matheus Alves de Almeida]

With the iomap conversion of buffered I/O complete, the ->mmu_private
inode field is no longer necessary.

Replace remaining users with ->i_size checks. Remove the DIO end callback
as DIO now checks ->i_size directly to reject extending writes.

Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
---
 fs/fat/cache.c | 15 ++-------------
 fs/fat/dir.c   |  1 -
 fs/fat/fat.h   |  4 ----
 fs/fat/file.c  | 23 +++++------------------
 fs/fat/inode.c | 10 +++-------
 fs/fat/iomap.c | 29 +++--------------------------
 6 files changed, 13 insertions(+), 69 deletions(-)

diff --git a/fs/fat/cache.c b/fs/fat/cache.c
index f545e1db1d47..89c950f87493 100644
--- a/fs/fat/cache.c
+++ b/fs/fat/cache.c
@@ -377,19 +377,8 @@ static int is_exceed_eof(struct inode *inode, sector_t sector,
 	const unsigned char blocksize_bits = sb->s_blocksize_bits;
 
 	*last_block = (i_size_read(inode) + (blocksize - 1)) >> blocksize_bits;
-	if (sector >= *last_block) {
-		if (!create)
-			return 1;
-
-		/*
-		 * ->mmu_private can access on only allocation path.
-		 * (caller must hold ->i_mutex)
-		 */
-		*last_block = (MSDOS_I(inode)->mmu_private + (blocksize - 1))
-			>> blocksize_bits;
-		if (sector >= *last_block)
-			return 1;
-	}
+	if (sector >= *last_block && !create)
+		return 1;
 
 	return 0;
 }
diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 35bdb62944a2..9996bd121e07 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -1443,7 +1443,6 @@ int fat_add_entries(struct inode *dir, void *slots, int nr_slots,
 				& ~((loff_t)sbi->cluster_size - 1);
 		}
 		dir->i_size += nr_cluster << sbi->cluster_bits;
-		MSDOS_I(dir)->mmu_private += nr_cluster << sbi->cluster_bits;
 	}
 	sinfo->slot_off = pos;
 	sinfo->de = de;
diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 78c6fb5cdc46..962f6f6b7717 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -122,9 +122,6 @@ struct msdos_inode_info {
 	/* for avoiding the race between fat_free() and fat_get_cluster() */
 	unsigned int cache_valid_id;
 
-	/* NOTE: mmu_private is 64bits, so must hold ->i_mutex to access */
-	loff_t mmu_private;	/* physically allocated size */
-
 	int i_start;		/* first cluster or 0 */
 	int i_logstart;		/* logical first cluster */
 	int i_attrs;		/* unused attribute bits */
@@ -493,7 +490,6 @@ extern const struct export_operations fat_export_ops_nostale;
 extern const struct iomap_ops fat_iomap_ops;
 extern const struct iomap_ops fat_iomap_bmap_ops;
 extern const struct iomap_ops fat_iomap_write_ops;
-extern const struct iomap_dio_ops fat_iomap_write_dio_ops;
 extern const struct iomap_writeback_ops fat_writeback_ops;
 
 /* helper for printk */
diff --git a/fs/fat/file.c b/fs/fat/file.c
index 723b671d5b4b..ac6329d36062 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -235,7 +235,6 @@ static ssize_t fat_file_dio_write_iter(struct kiocb *iocb,
 				       struct iov_iter *iter)
 {
 	struct inode *inode = file_inode(iocb->ki_filp);
-	struct msdos_inode_info *mi = MSDOS_I(inode);
 	unsigned int dio_flags = 0;
 	ssize_t ret;
 
@@ -245,7 +244,7 @@ static ssize_t fat_file_dio_write_iter(struct kiocb *iocb,
 		goto out;
 
 	/* We do not support DIO writes which would need zeroing */
-	if (iocb->ki_pos + iov_iter_count(iter) > mi->mmu_private) {
+	if (iocb->ki_pos + iov_iter_count(iter) > i_size_read(inode)) {
 		ret = -ENOTBLK;
 		goto out;
 	}
@@ -254,14 +253,13 @@ static ssize_t fat_file_dio_write_iter(struct kiocb *iocb,
 	if (ret)
 		goto out;
 
-	/* Wait under the inode lock for size updates and partial-block zeroing. */
-	if (iocb->ki_pos + iov_iter_count(iter) > i_size_read(inode) ||
-	    !IS_ALIGNED(iocb->ki_pos | iov_iter_alignment(iter),
+	/* Wait under the inode lock for unaligned writes */
+	if (!IS_ALIGNED(iocb->ki_pos | iov_iter_alignment(iter),
 			i_blocksize(inode)))
 		dio_flags |= IOMAP_DIO_FORCE_WAIT;
 
-	ret = iomap_dio_rw(iocb, iter, &fat_iomap_ops,
-			   &fat_iomap_write_dio_ops, dio_flags, NULL, 0);
+	ret = iomap_dio_rw(iocb, iter, &fat_iomap_ops, NULL, dio_flags,
+			   NULL, 0);
 out:
 	inode_unlock(inode);
 	return ret;
@@ -402,7 +400,6 @@ int fat_preallocate(struct inode *inode, loff_t new_size)
 
 static int fat_zero_and_extend(struct inode *inode, loff_t new_size)
 {
-	struct msdos_inode_info *mi = MSDOS_I(inode);
 	struct address_space *mapping = inode->i_mapping;
 	loff_t pos = i_size_read(inode), fpos, next;
 	loff_t old_size = pos;
@@ -439,7 +436,6 @@ static int fat_zero_and_extend(struct inode *inode, loff_t new_size)
 
 		folio_zero_segment(folio, from, to);
 		folio_mark_dirty(folio);
-		mi->mmu_private = max(mi->mmu_private, next);
 		i_size_write(inode, next);
 
 		folio_unlock(folio);
@@ -460,7 +456,6 @@ static int fat_cont_expand(struct inode *inode, loff_t new_size)
 	struct msdos_inode_info *mi = MSDOS_I(inode);
 	loff_t old_alloc = inode_get_bytes(inode);
 	loff_t old_size = i_size_read(inode);
-	loff_t old_mmu_private = mi->mmu_private;
 	int err;
 
 	err = inode_newsize_ok(inode, new_size);
@@ -478,7 +473,6 @@ static int fat_cont_expand(struct inode *inode, loff_t new_size)
 		filemap_invalidate_lock(inode->i_mapping);
 		truncate_setsize(inode, old_size);
 		fat_truncate_blocks(inode, old_alloc);
-		mi->mmu_private = old_mmu_private;
 		filemap_invalidate_unlock(inode->i_mapping);
 		return err;
 	}
@@ -605,13 +599,6 @@ void fat_truncate_blocks(struct inode *inode, loff_t offset)
 	const unsigned int cluster_size = sbi->cluster_size;
 	int nr_clusters;
 
-	/*
-	 * This protects against truncating a file bigger than it was then
-	 * trying to write into the hole.
-	 */
-	if (MSDOS_I(inode)->mmu_private > offset)
-		MSDOS_I(inode)->mmu_private = offset;
-
 	nr_clusters = (offset + (cluster_size - 1)) >> sbi->cluster_bits;
 
 	fat_free(inode, nr_clusters);
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 9df10e5a1ede..cb2316062bb3 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -356,7 +356,6 @@ int fat_fill_inode(struct inode *inode, struct msdos_dir_entry *de)
 		error = fat_calc_dir_size(inode);
 		if (error < 0)
 			return error;
-		MSDOS_I(inode)->mmu_private = inode->i_size;
 
 		set_nlink(inode, fat_subdirs(inode));
 
@@ -375,7 +374,6 @@ int fat_fill_inode(struct inode *inode, struct msdos_dir_entry *de)
 		inode->i_op = &fat_file_inode_operations;
 		inode->i_fop = &fat_file_operations;
 		inode->i_mapping->a_ops = &fat_aops;
-		MSDOS_I(inode)->mmu_private = inode->i_size;
 	}
 	if (de->attr & ATTR_SYS) {
 		if (sbi->options.sys_immutable)
@@ -486,11 +484,11 @@ static void fat_free_eofblocks(struct inode *inode)
 {
 	/* Release unwritten fallocated blocks on inode eviction. */
 	if ((inode->i_blocks << 9) >
-			round_up(MSDOS_I(inode)->mmu_private,
-				MSDOS_SB(inode->i_sb)->cluster_size)) {
+			round_up(i_size_read(inode),
+				 MSDOS_SB(inode->i_sb)->cluster_size)) {
 		int err;
 
-		fat_truncate_blocks(inode, MSDOS_I(inode)->mmu_private);
+		fat_truncate_blocks(inode, i_size_read(inode));
 		/* Fallocate results in updating the i_start/iogstart
 		 * for the zero byte file. So, make it return to
 		 * original state during evict and commit it to avoid
@@ -612,7 +610,6 @@ static struct inode *fat_alloc_inode(struct super_block *sb)
 		return NULL;
 
 	/* Zeroing to allow iput() even if partial initialized inode. */
-	ei->mmu_private = 0;
 	ei->i_start = 0;
 	ei->i_logstart = 0;
 	ei->i_attrs = 0;
@@ -1194,7 +1191,6 @@ static int fat_read_root(struct inode *inode)
 	inode->i_blocks = ((inode->i_size + (sbi->cluster_size - 1))
 			   & ~((loff_t)sbi->cluster_size - 1)) >> 9;
 	MSDOS_I(inode)->i_logstart = 0;
-	MSDOS_I(inode)->mmu_private = inode->i_size;
 
 	fat_save_attrs(inode, ATTR_DIR);
 	inode_set_mtime_to_ts(inode,
diff --git a/fs/fat/iomap.c b/fs/fat/iomap.c
index cfeaea5d352e..893d94f6b70c 100644
--- a/fs/fat/iomap.c
+++ b/fs/fat/iomap.c
@@ -83,12 +83,9 @@ static int fat_iomap_end_write(struct inode *inode, loff_t offset,
 {
 	struct msdos_inode_info *mi = MSDOS_I(inode);
 
-	if (written > 0) {
-		mi->mmu_private = max(mi->mmu_private, offset + written);
-		if (!(mi->i_attrs & ATTR_ARCH)) {
-			mi->i_attrs |= ATTR_ARCH;
-			mark_inode_dirty(inode);
-		}
+	if (written > 0 && !(mi->i_attrs & ATTR_ARCH)) {
+		mi->i_attrs |= ATTR_ARCH;
+		mark_inode_dirty(inode);
 	}
 
 	if (iomap->flags & IOMAP_F_SIZE_CHANGED)
@@ -160,26 +157,6 @@ const struct iomap_ops fat_iomap_bmap_ops = {
 	.iomap_next = fat_iomap_next_bmap
 };
 
-static int fat_dio_write_end_io(struct kiocb *iocb, ssize_t size, int error,
-				unsigned int flags)
-{
-	struct inode *inode = file_inode(iocb->ki_filp);
-
-	if (error)
-		return error;
-
-	if (size && i_size_read(inode) < iocb->ki_pos + size) {
-		i_size_write(inode, iocb->ki_pos + size);
-		mark_inode_dirty(inode);
-	}
-
-	return 0;
-}
-
-const struct iomap_dio_ops fat_iomap_write_dio_ops = {
-	.end_io = fat_dio_write_end_io
-};
-
 static ssize_t fat_writeback_range(struct iomap_writepage_ctx *wpc,
 				   struct folio *folio, u64 offset,
 				   unsigned int len, u64 end_pos)
-- 
2.43.0

[RFC PATCH 8/8] fat: add multi-cluster allocation for write

[Matheus Alves de Almeida]

The iomap code expands the file cluster-by-cluster, requiring many calls
into the allocator.

Implement multi-cluster allocation on write. To preserve short write
semantics, allow allocations of chains smaller than the requested size
in fat_alloc_chain().

Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
---
 fs/fat/fat.h    |  6 ++++--
 fs/fat/fatent.c | 33 ++++++++++++++++++++++-----------
 fs/fat/file.c   |  2 +-
 fs/fat/inode.c  |  4 ++--
 fs/fat/iomap.c  | 12 ++++++------
 5 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 962f6f6b7717..c953b2fd9c69 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -393,7 +393,8 @@ extern int fat_ent_write(struct inode *inode, struct fat_entry *fatent,
 			 int new, int wait);
 extern int fat_alloc_clusters(struct inode *inode, int *cluster,
 			      int nr_cluster);
-int fat_alloc_chain(struct inode *inode, int *cluster, int nr_cluster);
+int fat_alloc_chain(struct inode *inode, int *cluster, int *nr_cluster,
+		    bool allow_partial);
 extern int fat_free_clusters(struct inode *inode, int cluster);
 extern int fat_count_free_clusters(struct super_block *sb);
 extern int fat_trim_fs(struct inode *inode, struct fstrim_range *range);
@@ -439,7 +440,8 @@ static inline unsigned long fat_dir_hash(int logstart)
 {
 	return hash_32(logstart, FAT_HASH_BITS);
 }
-extern int fat_add_clusters(struct inode *inode, int nr_cluster);
+
+int fat_add_clusters(struct inode *inode, int nr_cluster, bool allow_partial);
 
 /* fat/misc.c */
 extern __printf(3, 4) __cold
diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
index 46899efa3cf8..6fb99a8759b8 100644
--- a/fs/fat/fatent.c
+++ b/fs/fat/fatent.c
@@ -538,8 +538,8 @@ static void fat_clean_bdev_aliases(struct super_block *sb,
 			   (sector_t)count * sbi->sec_per_clus);
 }
 
-static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluster,
-				bool return_all)
+static int __fat_alloc_clusters(struct inode *inode, int *cluster, int *nr_cluster,
+				bool return_all, bool allow_partial)
 {
 	struct super_block *sb = inode->i_sb;
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
@@ -547,12 +547,18 @@ static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluste
 	struct fat_entry fatent, prev_ent;
 	struct buffer_head *bhs[MAX_BUF_PER_PAGE];
 	int i, count, err, nr_bhs, idx_clus, cont_start, cont_len;
+	int nr_alloc = *nr_cluster;
 
 	lock_fat(sbi);
-	if (sbi->free_clusters != -1 && sbi->free_clus_valid &&
-	    sbi->free_clusters < nr_cluster) {
-		unlock_fat(sbi);
-		return -ENOSPC;
+	if (sbi->free_clusters != -1 && sbi->free_clus_valid) {
+		if (sbi->free_clusters == 0 ||
+		    (!allow_partial && sbi->free_clusters < nr_alloc)) {
+			unlock_fat(sbi);
+			return -ENOSPC;
+		}
+		if (allow_partial)
+			nr_alloc = min_t(unsigned int, nr_alloc,
+					 sbi->free_clusters);
 	}
 
 	cont_len = 0;
@@ -606,7 +612,7 @@ static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluste
 				}
 				fat_collect_bhs(bhs, &nr_bhs, &fatent);
 
-				if (idx_clus == nr_cluster)
+				if (idx_clus == nr_alloc)
 					goto out;
 
 				/*
@@ -627,7 +633,8 @@ static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluste
 	/* Couldn't allocate the free entries */
 	sbi->free_clusters = 0;
 	sbi->free_clus_valid = 1;
-	err = -ENOSPC;
+	if (!allow_partial || !idx_clus)
+		err = -ENOSPC;
 
 out:
 	if (cont_len)
@@ -647,17 +654,21 @@ static int __fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluste
 	if (err && idx_clus)
 		fat_free_clusters(inode, cluster[0]);
 
+	if (!err)
+		*nr_cluster = idx_clus;
 	return err;
 }
 
 int fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluster)
 {
-	return __fat_alloc_clusters(inode, cluster, nr_cluster, true);
+	return __fat_alloc_clusters(inode, cluster, &nr_cluster, true, false);
 }
 
-int fat_alloc_chain(struct inode *inode, int *cluster, int nr_cluster)
+int fat_alloc_chain(struct inode *inode, int *cluster, int *nr_cluster,
+		    bool allow_partial)
 {
-	return __fat_alloc_clusters(inode, cluster, nr_cluster, false);
+	return __fat_alloc_clusters(inode, cluster, nr_cluster, false,
+				    allow_partial);
 }
 
 int fat_free_clusters(struct inode *inode, int cluster)
diff --git a/fs/fat/file.c b/fs/fat/file.c
index ac6329d36062..a7349b2b94ee 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -395,7 +395,7 @@ int fat_preallocate(struct inode *inode, loff_t new_size)
 		     sbi->cluster_bits;
 
 	/* Allocate the clusters. We are not zeroing them out */
-	return fat_add_clusters(inode, nr_cluster);
+	return fat_add_clusters(inode, nr_cluster, false);
 }
 
 static int fat_zero_and_extend(struct inode *inode, loff_t new_size)
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index cb2316062bb3..6896c0c0dec9 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -101,11 +101,11 @@ static struct fat_floppy_defaults {
 },
 };
 
-int fat_add_clusters(struct inode *inode, int nr_cluster)
+int fat_add_clusters(struct inode *inode, int nr_cluster, bool allow_partial)
 {
 	int err, cluster;
 
-	err = fat_alloc_chain(inode, &cluster, nr_cluster);
+	err = fat_alloc_chain(inode, &cluster, &nr_cluster, allow_partial);
 	if (err)
 		return err;
 	/* FIXME: this cluster should be added after data of this
diff --git a/fs/fat/iomap.c b/fs/fat/iomap.c
index 893d94f6b70c..581fa609ae18 100644
--- a/fs/fat/iomap.c
+++ b/fs/fat/iomap.c
@@ -98,6 +98,7 @@ static int fat_iomap_begin_write(struct inode *inode, loff_t offset,
 				 loff_t length, unsigned int flags,
 				 struct iomap *iomap, struct iomap *srcmap)
 {
+	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
 	loff_t curr_size = i_size_read(inode);
 	loff_t old_alloc = inode_get_bytes(inode);
 	int err;
@@ -106,12 +107,11 @@ static int fat_iomap_begin_write(struct inode *inode, loff_t offset,
 	if (WARN_ONCE(offset > curr_size, "Write starting past EOF"))
 		return -EOPNOTSUPP;
 
-	if (offset == curr_size) {
-		/*
-		 * TODO: allocate multiple blocks while allowing
-		 * short writes
-		 */
-		err = fat_preallocate(inode, offset + 1);
+	if (offset == curr_size && offset >= old_alloc) {
+		int nr_cluster = (offset + length - old_alloc +
+				  sbi->cluster_size - 1) >> sbi->cluster_bits;
+
+		err = fat_add_clusters(inode, nr_cluster, true);
 		if (err)
 			goto err_truncate;
 	}
-- 
2.43.0