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@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxxxxx>