Re: [PATCH 1/2] btrfs: catch cow on deleting snapshots

[David Sterba <dsterba@suse.cz>]

On Fri, Nov 30, 2018 at 12:19:18PM -0500, Josef Bacik wrote:
> On Fri, Nov 30, 2018 at 05:14:54PM +0000, Filipe Manana wrote:
> > On Fri, Nov 30, 2018 at 4:53 PM Josef Bacik <josef@toxicpanda.com> wrote:
> > >
> > > From: Josef Bacik <jbacik@fb.com>
> > >
> > > When debugging some weird extent reference bug I suspected that we were
> > > changing a snapshot while we were deleting it, which could explain my
> > > bug.  This was indeed what was happening, and this patch helped me
> > > verify my theory.  It is never correct to modify the snapshot once it's
> > > being deleted, so mark the root when we are deleting it and make sure we
> > > complain about it when it happens.
> > >
> > > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > > ---
> > >  fs/btrfs/ctree.c       | 3 +++
> > >  fs/btrfs/ctree.h       | 1 +
> > >  fs/btrfs/extent-tree.c | 9 +++++++++
> > >  3 files changed, 13 insertions(+)
> > >
> > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> > > index 5912a97b07a6..5f82f86085e8 100644
> > > --- a/fs/btrfs/ctree.c
> > > +++ b/fs/btrfs/ctree.c
> > > @@ -1440,6 +1440,9 @@ noinline int btrfs_cow_block(struct btrfs_trans_handle *trans,
> > >         u64 search_start;
> > >         int ret;
> > >
> > > +       if (test_bit(BTRFS_ROOT_DELETING, &root->state))
> > > +               WARN(1, KERN_CRIT "cow'ing blocks on a fs root thats being dropped\n");
> > 
> > Please use btrfs_warn(), it makes sure we use a consistent message
> > style, identifies the fs, etc.
> > Also, "thats" should be "that is" or "that's".
> > 
> 
> Ah yeah, I was following the other convention in there but we should probably
> convert all of those to btrfs_warn.  I'll fix the grammer thing as well, just a
> leftover from the much less code of conduct friendly message I originally had
> there.  Thanks,

Committed with the following fixup:

-               WARN(1, KERN_CRIT "cow'ing blocks on a fs root thats being dropped\n");
+               btrfs_error(fs_info,
+                       "COW'ing blocks on a fs root that's being dropped");