[PATCH v7 3/4] fat: permit to return phy block number by fibmap in fallocated region

[PATCH v7 3/4] fat: permit to return phy block number by fibmap in fallocated region

[Namjae Jeon]

Make the fibmap call return the proper physical block number for any
offset request in the fallocated range.

Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
---
 fs/fat/cache.c | 38 +++++++++++++++++++++++++-------------
 fs/fat/fat.h   |  3 +++
 fs/fat/inode.c | 33 ++++++++++++++++++++++++++++++++-
 3 files changed, 60 insertions(+), 14 deletions(-)

diff --git a/fs/fat/cache.c b/fs/fat/cache.c
index 91ad9e1..d3dd5ba 100644
--- a/fs/fat/cache.c
+++ b/fs/fat/cache.c
@@ -303,6 +303,29 @@ static int fat_bmap_cluster(struct inode *inode, int cluster)
 	return dclus;
 }
 
+int fat_get_mapped_cluster(struct inode *inode, sector_t sector,
+			   sector_t last_block,
+			   unsigned long *mapped_blocks, sector_t *bmap)
+{
+	struct super_block *sb = inode->i_sb;
+	struct msdos_sb_info *sbi = MSDOS_SB(sb);
+	int cluster, offset;
+
+	cluster = sector >> (sbi->cluster_bits - sb->s_blocksize_bits);
+	offset  = sector & (sbi->sec_per_clus - 1);
+	cluster = fat_bmap_cluster(inode, cluster);
+	if (cluster < 0)
+		return cluster;
+	else if (cluster) {
+		*bmap = fat_clus_to_blknr(sbi, cluster) + offset;
+		*mapped_blocks = sbi->sec_per_clus - offset;
+		if (*mapped_blocks > last_block - sector)
+			*mapped_blocks = last_block - sector;
+	}
+
+	return 0;
+}
+
 int fat_bmap(struct inode *inode, sector_t sector, sector_t *phys,
 	     unsigned long *mapped_blocks, int create)
 {
@@ -311,7 +334,6 @@ int fat_bmap(struct inode *inode, sector_t sector, sector_t *phys,
 	const unsigned long blocksize = sb->s_blocksize;
 	const unsigned char blocksize_bits = sb->s_blocksize_bits;
 	sector_t last_block;
-	int cluster, offset;
 
 	*phys = 0;
 	*mapped_blocks = 0;
@@ -338,16 +360,6 @@ int fat_bmap(struct inode *inode, sector_t sector, sector_t *phys,
 			return 0;
 	}
 
-	cluster = sector >> (sbi->cluster_bits - sb->s_blocksize_bits);
-	offset  = sector & (sbi->sec_per_clus - 1);
-	cluster = fat_bmap_cluster(inode, cluster);
-	if (cluster < 0)
-		return cluster;
-	else if (cluster) {
-		*phys = fat_clus_to_blknr(sbi, cluster) + offset;
-		*mapped_blocks = sbi->sec_per_clus - offset;
-		if (*mapped_blocks > last_block - sector)
-			*mapped_blocks = last_block - sector;
-	}
-	return 0;
+	return fat_get_mapped_cluster(inode, sector, last_block, mapped_blocks,
+				      phys);
 }
diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index df72581..e59eb6e 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -288,6 +288,9 @@ 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);
+extern int fat_get_mapped_cluster(struct inode *inode, sector_t sector,
+				  sector_t last_block,
+				  unsigned long *mapped_blocks, sector_t *bmap);
 extern int fat_bmap(struct inode *inode, sector_t sector, sector_t *phys,
 		    unsigned long *mapped_blocks, int create);
 
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 164d2bc..1956dae 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -288,13 +288,44 @@ static ssize_t fat_direct_IO(int rw, struct kiocb *iocb,
 	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, last_block;
+	unsigned long mapped_blocks;
+
+	BUG_ON(create != 0);
+
+	last_block = inode->i_blocks >> (sb->s_blocksize_bits - 9);
+
+	if (iblock >= last_block)
+		return 0;
+
+	err = fat_get_mapped_cluster(inode, iblock, last_block, &mapped_blocks,
+		&bmap);
+	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);
+	blocknr = generic_block_bmap(mapping, block, fat_get_block_bmap);
 	up_read(&MSDOS_I(mapping->host)->truncate_lock);
 
 	return blocknr;
-- 
1.7.11-rc0

Re: [PATCH v7 3/4] fat: permit to return phy block number by fibmap in fallocated region

[OGAWA Hirofumi]

Namjae Jeon <namjae.jeon@samsung.com> writes:

Hi,

Sorry for long delay.

> +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, last_block;
> +	unsigned long mapped_blocks;
> +
> +	BUG_ON(create != 0);
> +
> +	last_block = inode->i_blocks >> (sb->s_blocksize_bits - 9);
> +
> +	if (iblock >= last_block)
> +		return 0;
> +
> +	err = fat_get_mapped_cluster(inode, iblock, last_block, &mapped_blocks,
> +		&bmap);
> +	if (err)
> +		return err;

Probably, we are missing rootdir check here? If it is fat12/fat16,
rootdir has to treat as special, because it doesn't have cluster chain.

So, how about to pass flags to fat_bmap()? For example,

	/* Check EOF for excluding bmap() ioctl path */
	static int is_exceed_eof()
	{
		...
	}

	fat_bmap(..., from_bmap)
	{
		...

		if (!from_bmap && is_exceed_eof(inode, sector, create))
			return 0;

		...
	}

or such.

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

RE: [PATCH v7 3/4] fat: permit to return phy block number by fibmap in fallocated region

[Namjae Jeon]

> 
> Hi,
> 
> Sorry for long delay.
> 
> > +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, last_block;
> > +	unsigned long mapped_blocks;
> > +
> > +	BUG_ON(create != 0);
> > +
> > +	last_block = inode->i_blocks >> (sb->s_blocksize_bits - 9);
> > +
> > +	if (iblock >= last_block)
> > +		return 0;
> > +
> > +	err = fat_get_mapped_cluster(inode, iblock, last_block, &mapped_blocks,
> > +		&bmap);
> > +	if (err)
> > +		return err;
> 
Hi OGAWA.
> Probably, we are missing rootdir check here? If it is fat12/fat16,
> rootdir has to treat as special, because it doesn't have cluster chain.
> 
> So, how about to pass flags to fat_bmap()? For example,
> 
> 	/* Check EOF for excluding bmap() ioctl path */
> 	static int is_exceed_eof()
> 	{
> 		...
> 	}
> 
> 	fat_bmap(..., from_bmap)
> 	{
> 		...
> 
> 		if (!from_bmap && is_exceed_eof(inode, sector, create))
> 			return 0;
> 
> 		...
> 	}
> 
> or such.
Right, root dir check is missing, we need to put it here.
And I will change it as you suggest.

Thanks for your review:)
> 
> Thanks.
> --
> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>