thr3ads.net - Btrfs devel - [PATCH] Btrfs: Fix NOCOW file attribute [Jun 2012]

If this information is useful, please help other people find it:
Share via:

Andrei Popa

2012-Jun-16 12:37 UTC

[PATCH] Btrfs: Fix NOCOW file attribute

Btrfs doesn''t honour correctly the NOCOW file attribute, this patch
fixes this.
NOCOW per file support has been added in e2fsprogs-1.42.4

Andrei Popa (1):
  Btrfs: Fix NOCOW file attribute

 fs/btrfs/ioctl.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

-- 
1.7.3.4

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

Andrei Popa

2012-Jun-16 12:37 UTC

head link

[PATCH] Btrfs: Fix NOCOW file attribute

Adds BTRFS_INODE_NODATASUM to inode flags when setting NOCOW for a file (chattr
+C file).
In btrfs, NOCOW implies NODATASUM and without setting NODATASUM, btrfs
doesn''t honour correctly the NOCOW attribute.

Signed-off-by: Andrei Popa <andrei.popa@i-neo.ro>
---
 fs/btrfs/ioctl.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 0e92e57..8a7be76 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -139,7 +139,7 @@ void btrfs_inherit_iflags(struct inode *inode, struct inode
*dir)
 	}
 
 	if (flags & BTRFS_INODE_NODATACOW)
-		BTRFS_I(inode)->flags |= BTRFS_INODE_NODATACOW;
+		BTRFS_I(inode)->flags |= BTRFS_INODE_NODATACOW | BTRFS_INODE_NODATASUM;
 
 	btrfs_update_iflags(inode);
 }
@@ -236,7 +236,7 @@ static int btrfs_ioctl_setflags(struct file *file, void
__user *arg)
 	else
 		ip->flags &= ~BTRFS_INODE_DIRSYNC;
 	if (flags & FS_NOCOW_FL)
-		ip->flags |= BTRFS_INODE_NODATACOW;
+		ip->flags |= BTRFS_INODE_NODATACOW | BTRFS_INODE_NODATASUM;
 	else
 		ip->flags &= ~BTRFS_INODE_NODATACOW;
 
-- 
1.7.3.4

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

Andrei Popa

2012-Jun-16 12:42 UTC

head link

Re: [PATCH] Btrfs: Fix NOCOW file attribute

Hi,

I''ve used the following to test the patch:
rm a
touch a
chattr +C a
dd if=/dev/zero of=a bs=32k conv=notrunc,nocreat count=1
sync
filefrag -v a
dd if=/dev/zero of=a bs=32k conv=notrunc,nocreat count=1
sync
filefrag -v a

Before the patch:
ierdnac-hp btrfs # ./test
+ rm a
+ touch a
+ chattr +C a
+ dd if=/dev/zero of=a bs=32k conv=notrunc,nocreat count=1
+ sync
+ filefrag -v a
Filesystem type is: 9123683e
File size of a is 32768 (8 blocks, blocksize 4096)
ext logical physical expected length flags
   0       0  1473536               8 eof
a: 1 extent found
+ dd if=/dev/zero of=a bs=32k conv=notrunc,nocreat count=1
+ sync
+ filefrag -v a
Filesystem type is: 9123683e
File size of a is 32768 (8 blocks, blocksize 4096)
ext logical physical expected length flags
   0       0  1473560               8 eof
a: 1 extent found
ierdnac-hp btrfs # 

After the patch:
ierdnac-hp btrfs # ./test
+ rm a
+ touch a
+ chattr +C a
+ dd if=/dev/zero of=a bs=32k conv=notrunc,nocreat count=1
1+0 records in
1+0 records out
32768 bytes (33 kB) copied, 3.651e-05 s, 898 MB/s
+ sync
+ filefrag -v a
Filesystem type is: 9123683e
File size of a is 32768 (8 blocks, blocksize 4096)
ext logical physical expected length flags
   0       0  1473560               8 eof
a: 1 extent found
+ dd if=/dev/zero of=a bs=32k conv=notrunc,nocreat count=1
1+0 records in
1+0 records out
32768 bytes (33 kB) copied, 4.6666e-05 s, 702 MB/s
+ sync
+ filefrag -v a
Filesystem type is: 9123683e
File size of a is 32768 (8 blocks, blocksize 4096)
ext logical physical expected length flags
   0       0  1473560               8 eof
a: 1 extent found
ierdnac-hp btrfs # 

My question is what is the expected behaviour for setting the chattr
after the file is filed with bytes like this:
rm a
touch a
# chattr +C a -> before chattr was here and we move it below
dd if=/dev/zero of=a bs=32k conv=notrunc,nocreat count=1
sync
filefrag -v a
chattr +C a
dd if=/dev/zero of=a bs=32k conv=notrunc,nocreat count=1
sync
filefrag -v a

Because in this case btrfs doesn''t honour the NOCOW attribute:
+ rm a
+ touch a
+ dd if=/dev/zero of=a bs=32k conv=notrunc,nocreat count=1
1+0 records in
1+0 records out
32768 bytes (33 kB) copied, 3.5127e-05 s, 933 MB/s
+ sync
+ filefrag -v a
Filesystem type is: 9123683e
File size of a is 32768 (8 blocks, blocksize 4096)
ext logical physical expected length flags
   0       0  1473544               8 eof
a: 1 extent found
+ chattr +C a
+ dd if=/dev/zero of=a bs=32k conv=notrunc,nocreat count=1
1+0 records in
1+0 records out
32768 bytes (33 kB) copied, 3.4169e-05 s, 959 MB/s
+ sync
+ filefrag -v a
Filesystem type is: 9123683e
File size of a is 32768 (8 blocks, blocksize 4096)
ext logical physical expected length flags
   0       0  1473536               8 eof
a: 1 extent found

It''s supposed to honour the NOCOW attribute or not ?

On Sat, 2012-06-16 at 15:37 +0300, Andrei Popa wrote:> Adds BTRFS_INODE_NODATASUM to inode flags when setting NOCOW for a file
(chattr +C file).
> In btrfs, NOCOW implies NODATASUM and without setting NODATASUM, btrfs
doesn''t honour correctly the NOCOW attribute.
> 
> Signed-off-by: Andrei Popa <andrei.popa@i-neo.ro>
> ---
>  fs/btrfs/ioctl.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 0e92e57..8a7be76 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -139,7 +139,7 @@ void btrfs_inherit_iflags(struct inode *inode, struct
inode *dir)
>  	}
>  
>  	if (flags & BTRFS_INODE_NODATACOW)
> -		BTRFS_I(inode)->flags |= BTRFS_INODE_NODATACOW;
> +		BTRFS_I(inode)->flags |= BTRFS_INODE_NODATACOW |
BTRFS_INODE_NODATASUM;
>  
>  	btrfs_update_iflags(inode);
>  }
> @@ -236,7 +236,7 @@ static int btrfs_ioctl_setflags(struct file *file, void
__user *arg)
>  	else
>  		ip->flags &= ~BTRFS_INODE_DIRSYNC;
>  	if (flags & FS_NOCOW_FL)
> -		ip->flags |= BTRFS_INODE_NODATACOW;
> +		ip->flags |= BTRFS_INODE_NODATACOW | BTRFS_INODE_NODATASUM;
>  	else
>  		ip->flags &= ~BTRFS_INODE_NODATACOW;
>  
-- 
Andrei Popa
(+4) 0741.57.80.90

David Sterba

2012-Jun-18 11:50 UTC

head link

Re: [PATCH] Btrfs: Fix NOCOW file attribute

On Sat, Jun 16, 2012 at 03:37:12PM +0300, Andrei Popa
wrote:> Adds BTRFS_INODE_NODATASUM to inode flags when setting NOCOW for a file
(chattr +C file).
> In btrfs, NOCOW implies NODATASUM and without setting NODATASUM, btrfs
doesn''t honour correctly the NOCOW attribute.
> 
> Signed-off-by: Andrei Popa <andrei.popa@i-neo.ro>
> ---
>  fs/btrfs/ioctl.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 0e92e57..8a7be76 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -139,7 +139,7 @@ void btrfs_inherit_iflags(struct inode *inode, struct
inode *dir)
>  	}
>  
>  	if (flags & BTRFS_INODE_NODATACOW)
> -		BTRFS_I(inode)->flags |= BTRFS_INODE_NODATACOW;
> +		BTRFS_I(inode)->flags |= BTRFS_INODE_NODATACOW |
BTRFS_INODE_NODATASUM;
>  
>  	btrfs_update_iflags(inode);
>  }
> @@ -236,7 +236,7 @@ static int btrfs_ioctl_setflags(struct file *file, void
__user *arg)
>  	else
>  		ip->flags &= ~BTRFS_INODE_DIRSYNC;
>  	if (flags & FS_NOCOW_FL)
> -		ip->flags |= BTRFS_INODE_NODATACOW;
> +		ip->flags |= BTRFS_INODE_NODATACOW | BTRFS_INODE_NODATASUM;
>  	else
>  		ip->flags &= ~BTRFS_INODE_NODATACOW;
>  
you cannot switch on NODATASUM for a file that already has extents that
are checksummed. The invariant here is that either all the extents or
none are checksummed.

Setting NODATASUM is safe only on a zero sized file, that''s what you
saw
in your test.

Theoretically it''s possible to add/remove all checksums from a given
file, but it''s a potentially longtime operation, the file has to be in
some intermediate state where the checksums partially exist but have to
be ignored (for the csum->nocsum) until the file is fully converted,
this brings more special cases to extent handling, it has to survive
power failure and remain consistent, and probably needs to be restarted
after next mount.


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

Btrfs devel - Jun 2012 - [PATCH] Btrfs: Fix NOCOW file attribute

[PATCH] Btrfs: Fix NOCOW file attribute

[PATCH] Btrfs: Fix NOCOW file attribute

Re: [PATCH] Btrfs: Fix NOCOW file attribute

Re: [PATCH] Btrfs: Fix NOCOW file attribute