Re: [PATCH 2/4] fat: return only first cluster when allocating

[OGAWA Hirofumi]

Matheus Alves de Almeida <matheus.aalmeida@xxxxxxxxxxxx> writes:

> fat_alloc_clusters() returns every allocated cluster number in an array.
> For larger allocations, like growing a file, this array would need to be
> very large.
>
> Make fat_alloc_clusters() return only the first allocated cluster in the
> chain. Update fat_add_new_entries() to follow the chain when it needs
> the second cluster.

This would be more common case than fallocate(), so penalize this part
would not be good.

Thanks.

> Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@xxxxxxxxxxxx>
> ---
>  fs/fat/dir.c    | 20 +++++++++++++++++++-
>  fs/fat/fat.h    |  2 +-
>  fs/fat/fatent.c | 17 +++++++++--------
>  3 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index 35bdb6294..672d5e392 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -1245,6 +1245,7 @@ static int fat_add_new_entries(struct inode *dir, void *slots, int nr_slots,
>  	sector_t blknr, start_blknr, last_blknr;
>  	unsigned long size, copy;
>  	int err, i, n, offset, cluster[2];
> +	struct fat_entry fatent;
>  
>  	/*
>  	 * The minimum cluster size is 512bytes, and maximum entry
> @@ -1255,10 +1256,26 @@ static int fat_add_new_entries(struct inode *dir, void *slots, int nr_slots,
>  	*nr_cluster = (size + (sbi->cluster_size - 1)) >> sbi->cluster_bits;
>  	BUG_ON(*nr_cluster > 2);
>  
> -	err = fat_alloc_clusters(dir, cluster, *nr_cluster);
> +	err = fat_alloc_clusters(dir, &cluster[0], *nr_cluster);
>  	if (err)
>  		goto error;
>  
> +	if (*nr_cluster > 1) {
> +		fatent_init(&fatent);
> +		cluster[1] = fat_ent_read(dir, &fatent, cluster[0]);
> +		fatent_brelse(&fatent);
> +		if (cluster[1] == FAT_ENT_EOF || cluster[1] == FAT_ENT_FREE) {
> +			fat_fs_error(sb,
> +				     "%s: invalid cluster chain (i_pos %lld)",
> +				     __func__, MSDOS_I(dir)->i_pos);
> +			cluster[1] = -EIO;
> +		}
> +		if (cluster[1] < 0) {
> +			err = cluster[1];
> +			goto error_ent;
> +		}
> +	}
> +
>  	/*
>  	 * First stage: Fill the directory entry.  NOTE: This cluster
>  	 * is not referenced from any inode yet, so updates order is
> @@ -1313,6 +1330,7 @@ static int fat_add_new_entries(struct inode *dir, void *slots, int nr_slots,
>  error_nomem:
>  	for (i = 0; i < n; i++)
>  		bforget(bhs[i]);
> +error_ent:
>  	fat_free_clusters(dir, cluster[0]);
>  error:
>  	return err;
> diff --git a/fs/fat/fat.h b/fs/fat/fat.h
> index 61338413d..75ac0239a 100644
> --- a/fs/fat/fat.h
> +++ b/fs/fat/fat.h
> @@ -393,7 +393,7 @@ extern int fat_ent_read(struct inode *inode, struct fat_entry *fatent,
>  			int entry);
>  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,
> +extern int fat_alloc_clusters(struct inode *inode, int *first_cluster,
>  			      int nr_cluster);
>  extern int fat_free_clusters(struct inode *inode, int cluster);
>  extern int fat_count_free_clusters(struct super_block *sb);
> diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
> index bfab8eab5..4970b1f14 100644
> --- a/fs/fat/fatent.c
> +++ b/fs/fat/fatent.c
> @@ -488,14 +488,14 @@ static int fat_flush_bhs(struct super_block *sb, struct buffer_head **bhs,
>  	return 0;
>  }
>  
> -int fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluster)
> +int fat_alloc_clusters(struct inode *inode, int *first_cluster, int nr_cluster)
>  {
>  	struct super_block *sb = inode->i_sb;
>  	struct msdos_sb_info *sbi = MSDOS_SB(sb);
>  	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, clusters_done;
>  
>  	lock_fat(sbi);
>  	if (sbi->free_clusters != -1 && sbi->free_clus_valid &&
> @@ -504,7 +504,7 @@ int fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluster)
>  		return -ENOSPC;
>  	}
>  
> -	err = nr_bhs = idx_clus = 0;
> +	err = nr_bhs = clusters_done = 0;
>  	count = FAT_START_ENT;
>  	fatent_init(&prev_ent);
>  	fatent_init(&fatent);
> @@ -531,8 +531,9 @@ int fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluster)
>  				if (sbi->free_clusters != -1)
>  					sbi->free_clusters--;
>  
> -				cluster[idx_clus] = entry;
> -				idx_clus++;
> +				if (!clusters_done)
> +					*first_cluster = entry;
> +				clusters_done++;
>  
>  				if (nr_bhs + fatent.nr_bhs > MAX_BUF_PER_PAGE) {
>  					err = fat_flush_bhs(sb, bhs, &nr_bhs,
> @@ -542,7 +543,7 @@ int fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluster)
>  				}
>  				fat_collect_bhs(bhs, &nr_bhs, &fatent);
>  
> -				if (idx_clus == nr_cluster)
> +				if (clusters_done == nr_cluster)
>  					goto out;
>  
>  				/*
> @@ -575,8 +576,8 @@ int fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluster)
>  	for (i = 0; i < nr_bhs; i++)
>  		brelse(bhs[i]);
>  
> -	if (err && idx_clus)
> -		fat_free_clusters(inode, cluster[0]);
> +	if (err && clusters_done)
> +		fat_free_clusters(inode, *first_cluster);
>  
>  	return err;
>  }

-- 
OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>