diff mbox

Btrfs: make sure NODATACOW also gets NODATASUM set

Message ID 1361478855-5059-1-git-send-email-jbacik@fusionio.com (mailing list archive)
State New, archived
Headers show

Commit Message

Josef Bacik Feb. 21, 2013, 8:34 p.m. UTC
A user reported hitting the BUG_ON() in btrfs_finished_ordered_io() where we had
csums on a NOCOW extent.  This can happen if we have NODATACOW set but not
NODATASUM set, which can happen in two cases, either we mount with -o nodatacow
and then write into preallocated space, or chattr +C a directory and move a file
into that directory.  This patch fixes up those two cases and now we should
uniformly have NODATACOW and NODATASUM set always.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fusionio.com>
---
 fs/btrfs/inode.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

Comments

Liu Bo Feb. 22, 2013, 1:43 a.m. UTC | #1
On Thu, Feb 21, 2013 at 03:34:15PM -0500, Josef Bacik wrote:
> A user reported hitting the BUG_ON() in btrfs_finished_ordered_io() where we had
> csums on a NOCOW extent.  This can happen if we have NODATACOW set but not
> NODATASUM set, which can happen in two cases, either we mount with -o nodatacow
> and then write into preallocated space, or chattr +C a directory and move a file
> into that directory.  This patch fixes up those two cases and now we should
> uniformly have NODATACOW and NODATASUM set always.  Thanks,

Reviewed-by: Liu Bo <bo.li.liu@oracle.com>

> 
> Signed-off-by: Josef Bacik <jbacik@fusionio.com>
> ---
>  fs/btrfs/inode.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 40d49da..5d7ceb5 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -5574,7 +5574,8 @@ static struct inode *btrfs_new_inode(struct btrfs_trans_handle *trans,
>  		if (btrfs_test_opt(root, NODATASUM))
>  			BTRFS_I(inode)->flags |= BTRFS_INODE_NODATASUM;
>  		if (btrfs_test_opt(root, NODATACOW))
> -			BTRFS_I(inode)->flags |= BTRFS_INODE_NODATACOW;
> +			BTRFS_I(inode)->flags |= BTRFS_INODE_NODATACOW |
> +				BTRFS_INODE_NODATASUM;
>  	}
>  
>  	insert_inode_hash(inode);
> @@ -8069,7 +8070,8 @@ static void fixup_inode_flags(struct inode *dir, struct inode *inode)
>  	struct btrfs_inode *b_inode = BTRFS_I(inode);
>  
>  	if (b_dir->flags & BTRFS_INODE_NODATACOW)
> -		b_inode->flags |= BTRFS_INODE_NODATACOW;
> +		b_inode->flags |= BTRFS_INODE_NODATACOW |
> +			BTRFS_INODE_NODATASUM;
>  	else
>  		b_inode->flags &= ~BTRFS_INODE_NODATACOW;
>  
> -- 
> 1.7.7.6
> 
> --
> 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
--
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
Marios Titas Feb. 22, 2013, 6:34 a.m. UTC | #2
A few weeks ago I reported a similar bug [1]. It has to do with file
renaming. Do you think that it is related? This patch doesn't seem to
help.

[1] http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg21640.html

On Thu, Feb 21, 2013 at 3:34 PM, Josef Bacik <jbacik@fusionio.com> wrote:
> A user reported hitting the BUG_ON() in btrfs_finished_ordered_io() where we had
> csums on a NOCOW extent.  This can happen if we have NODATACOW set but not
> NODATASUM set, which can happen in two cases, either we mount with -o nodatacow
> and then write into preallocated space, or chattr +C a directory and move a file
> into that directory.  This patch fixes up those two cases and now we should
> uniformly have NODATACOW and NODATASUM set always.  Thanks,
>
> Signed-off-by: Josef Bacik <jbacik@fusionio.com>
> ---
>  fs/btrfs/inode.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 40d49da..5d7ceb5 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -5574,7 +5574,8 @@ static struct inode *btrfs_new_inode(struct btrfs_trans_handle *trans,
>                 if (btrfs_test_opt(root, NODATASUM))
>                         BTRFS_I(inode)->flags |= BTRFS_INODE_NODATASUM;
>                 if (btrfs_test_opt(root, NODATACOW))
> -                       BTRFS_I(inode)->flags |= BTRFS_INODE_NODATACOW;
> +                       BTRFS_I(inode)->flags |= BTRFS_INODE_NODATACOW |
> +                               BTRFS_INODE_NODATASUM;
>         }
>
>         insert_inode_hash(inode);
> @@ -8069,7 +8070,8 @@ static void fixup_inode_flags(struct inode *dir, struct inode *inode)
>         struct btrfs_inode *b_inode = BTRFS_I(inode);
>
>         if (b_dir->flags & BTRFS_INODE_NODATACOW)
> -               b_inode->flags |= BTRFS_INODE_NODATACOW;
> +               b_inode->flags |= BTRFS_INODE_NODATACOW |
> +                       BTRFS_INODE_NODATASUM;
>         else
>                 b_inode->flags &= ~BTRFS_INODE_NODATACOW;
>
> --
> 1.7.7.6
>
> --
> 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
--
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
Liu Bo Feb. 22, 2013, 7:34 a.m. UTC | #3
On Fri, Feb 22, 2013 at 01:34:26AM -0500, Marios Titas wrote:
> A few weeks ago I reported a similar bug [1]. It has to do with file
> renaming. Do you think that it is related? This patch doesn't seem to
> help.

Yes, they're related, and I sent you a patch in [1] thread, could you check it?

thanks,
liubo

> 
> [1] http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg21640.html
> 
> On Thu, Feb 21, 2013 at 3:34 PM, Josef Bacik <jbacik@fusionio.com> wrote:
> > A user reported hitting the BUG_ON() in btrfs_finished_ordered_io() where we had
> > csums on a NOCOW extent.  This can happen if we have NODATACOW set but not
> > NODATASUM set, which can happen in two cases, either we mount with -o nodatacow
> > and then write into preallocated space, or chattr +C a directory and move a file
> > into that directory.  This patch fixes up those two cases and now we should
> > uniformly have NODATACOW and NODATASUM set always.  Thanks,
> >
> > Signed-off-by: Josef Bacik <jbacik@fusionio.com>
> > ---
> >  fs/btrfs/inode.c |    6 ++++--
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index 40d49da..5d7ceb5 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -5574,7 +5574,8 @@ static struct inode *btrfs_new_inode(struct btrfs_trans_handle *trans,
> >                 if (btrfs_test_opt(root, NODATASUM))
> >                         BTRFS_I(inode)->flags |= BTRFS_INODE_NODATASUM;
> >                 if (btrfs_test_opt(root, NODATACOW))
> > -                       BTRFS_I(inode)->flags |= BTRFS_INODE_NODATACOW;
> > +                       BTRFS_I(inode)->flags |= BTRFS_INODE_NODATACOW |
> > +                               BTRFS_INODE_NODATASUM;
> >         }
> >
> >         insert_inode_hash(inode);
> > @@ -8069,7 +8070,8 @@ static void fixup_inode_flags(struct inode *dir, struct inode *inode)
> >         struct btrfs_inode *b_inode = BTRFS_I(inode);
> >
> >         if (b_dir->flags & BTRFS_INODE_NODATACOW)
> > -               b_inode->flags |= BTRFS_INODE_NODATACOW;
> > +               b_inode->flags |= BTRFS_INODE_NODATACOW |
> > +                       BTRFS_INODE_NODATASUM;
> >         else
> >                 b_inode->flags &= ~BTRFS_INODE_NODATACOW;
> >
> > --
> > 1.7.7.6
> >
> > --
> > 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
> --
> 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
--
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
diff mbox

Patch

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 40d49da..5d7ceb5 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5574,7 +5574,8 @@  static struct inode *btrfs_new_inode(struct btrfs_trans_handle *trans,
 		if (btrfs_test_opt(root, NODATASUM))
 			BTRFS_I(inode)->flags |= BTRFS_INODE_NODATASUM;
 		if (btrfs_test_opt(root, NODATACOW))
-			BTRFS_I(inode)->flags |= BTRFS_INODE_NODATACOW;
+			BTRFS_I(inode)->flags |= BTRFS_INODE_NODATACOW |
+				BTRFS_INODE_NODATASUM;
 	}
 
 	insert_inode_hash(inode);
@@ -8069,7 +8070,8 @@  static void fixup_inode_flags(struct inode *dir, struct inode *inode)
 	struct btrfs_inode *b_inode = BTRFS_I(inode);
 
 	if (b_dir->flags & BTRFS_INODE_NODATACOW)
-		b_inode->flags |= BTRFS_INODE_NODATACOW;
+		b_inode->flags |= BTRFS_INODE_NODATACOW |
+			BTRFS_INODE_NODATASUM;
 	else
 		b_inode->flags &= ~BTRFS_INODE_NODATACOW;