Patchwork [2/3,v2] ext4: let us fully support punching hole feature in fallocate

login
register
mail settings
Submitter Zheng Liu
Date Nov. 19, 2012, 12:55 p.m.
Message ID <1353329718-2075-3-git-send-email-wenqing.lz@taobao.com>
Download mbox | patch
Permalink /patch/199996/
State Superseded
Headers show

Comments

Zheng Liu - Nov. 19, 2012, 12:55 p.m.
From: Zheng Liu <wenqing.lz@taobao.com>

After adding indirect punching hole feature, we need to enable it in fallocate.
For this purpose, some sanity checks need to be adjusted.  Currently we need to
check FALLOC_FL_PUNCH_HOLE flag before other sanity checks.

Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
---
 fs/ext4/extents.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
Guo Chao - Nov. 20, 2012, 6:35 a.m.
Hi, Zheng:

On Mon, Nov 19, 2012 at 08:55:17PM +0800, Zheng Liu wrote:
> From: Zheng Liu <wenqing.lz@taobao.com>
> 
> After adding indirect punching hole feature, we need to enable it in fallocate.
> For this purpose, some sanity checks need to be adjusted.  Currently we need to
> check FALLOC_FL_PUNCH_HOLE flag before other sanity checks.
> 
> Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
> ---
>  fs/ext4/extents.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 7011ac9..b43b3e9 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4420,13 +4420,6 @@ long ext4_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
>  	struct ext4_map_blocks map;
>  	unsigned int credits, blkbits = inode->i_blkbits;
> 
> -	/*
> -	 * currently supporting (pre)allocate mode for extent-based
> -	 * files _only_
> -	 */
> -	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
> -		return -EOPNOTSUPP;
> -
>  	/* Return error if mode is not supported */
>  	if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
>  		return -EOPNOTSUPP;

Checking these mode flags seems redundant here, VFS already checked them.
Maybe you can remove it by the way.

Regards,
Guo Chao

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zheng Liu - Nov. 20, 2012, 7:43 a.m.
On Tue, Nov 20, 2012 at 02:35:05PM +0800, Guo Chao wrote:
> Hi, Zheng:
> 
> On Mon, Nov 19, 2012 at 08:55:17PM +0800, Zheng Liu wrote:
> > From: Zheng Liu <wenqing.lz@taobao.com>
> > 
> > After adding indirect punching hole feature, we need to enable it in fallocate.
> > For this purpose, some sanity checks need to be adjusted.  Currently we need to
> > check FALLOC_FL_PUNCH_HOLE flag before other sanity checks.
> > 
> > Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
> > ---
> >  fs/ext4/extents.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> > index 7011ac9..b43b3e9 100644
> > --- a/fs/ext4/extents.c
> > +++ b/fs/ext4/extents.c
> > @@ -4420,13 +4420,6 @@ long ext4_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
> >  	struct ext4_map_blocks map;
> >  	unsigned int credits, blkbits = inode->i_blkbits;
> > 
> > -	/*
> > -	 * currently supporting (pre)allocate mode for extent-based
> > -	 * files _only_
> > -	 */
> > -	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
> > -		return -EOPNOTSUPP;
> > -
> >  	/* Return error if mode is not supported */
> >  	if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
> >  		return -EOPNOTSUPP;
> 
> Checking these mode flags seems redundant here, VFS already checked them.
> Maybe you can remove it by the way.

Yeah, I see.  Not only ext4, other filesystems also check it in themselves,
such as xfs, btrfs.  I am not very familiar with why we need to do this,
but IMHO a better way might be removing it from all filesystems in another
patch series.  I will send it out.  Thanks for your suggestion.

Regards,
                                                - Zheng
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 7011ac9..b43b3e9 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4420,13 +4420,6 @@  long ext4_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 	struct ext4_map_blocks map;
 	unsigned int credits, blkbits = inode->i_blkbits;
 
-	/*
-	 * currently supporting (pre)allocate mode for extent-based
-	 * files _only_
-	 */
-	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
-		return -EOPNOTSUPP;
-
 	/* Return error if mode is not supported */
 	if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
 		return -EOPNOTSUPP;
@@ -4434,6 +4427,13 @@  long ext4_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 	if (mode & FALLOC_FL_PUNCH_HOLE)
 		return ext4_punch_hole(file, offset, len);
 
+	/*
+	 * currently supporting (pre)allocate mode for extent-based
+	 * files _only_
+	 */
+	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
+		return -EOPNOTSUPP;
+
 	trace_ext4_fallocate_enter(inode, offset, len, mode);
 	map.m_lblk = offset >> blkbits;
 	/*