Re: [PATCH] Btrfs: fix possible panic on unmount

[Josef Bacik <josef@redhat.com>]

On Sat, Nov 14, 2009 at 03:56:28AM +0800, Yan, Zheng  wrote:
> On Sat, Nov 14, 2009 at 1:06 AM, Josef Bacik <josef@redhat.com> wrote=
:
> > We can race with the unmount of an fs and the stopping of a kthread=
 where we
> > will free the block group before we're done using it. =A0The reason=
 for this is
> > because we do not hold a reference on the block group while its cac=
hing, since
> > the allocator drops its reference once it exits or moves on to the =
next block
> > group. =A0This patch fixes the problem by taking a reference to the=
 block group
> > before we start caching and dropping it when we're done to make sur=
e all
> > accesses to the block group are safe. =A0Thanks,
> >
> > Signed-off-by: Josef Bacik <josef@redhat.com>
> > ---
> > =A0fs/btrfs/extent-tree.c | =A0 =A03 +++
> > =A01 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> > index 2a4cdce..197bc1b 100644
> > --- a/fs/btrfs/extent-tree.c
> > +++ b/fs/btrfs/extent-tree.c
> > @@ -399,6 +399,8 @@ err:
> >
> > =A0 =A0 =A0 =A0put_caching_control(caching_ctl);
> > =A0 =A0 =A0 =A0atomic_dec(&block_group->space_info->caching_threads=
);
> > + =A0 =A0 =A0 btrfs_put_block_group(block_group);
> > +
> > =A0 =A0 =A0 =A0return 0;
> > =A0}
> >
> > @@ -439,6 +441,7 @@ static int cache_block_group(struct btrfs_block=
_group_cache *cache)
> > =A0 =A0 =A0 =A0up_write(&fs_info->extent_commit_sem);
> >
> > =A0 =A0 =A0 =A0atomic_inc(&cache->space_info->caching_threads);
> > + =A0 =A0 =A0 atomic_inc(&cache->count);
> >
> > =A0 =A0 =A0 =A0tsk =3D kthread_run(caching_kthread, cache, "btrfs-c=
ache-%llu\n",
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0cache->key.objec=
tid);
> > --
>=20
> This won't work since btrfs_free_block_groups call kfree without
> checking the reference
> count. I think the correct way to fix the race is waiting until
> caching threads exit
>=20

We do already do that, but the thing is we mark the block group as bein=
g
finished, and then go on and do other things with it, hence the race.  =
The
correct thing to do is do a put in btrfs_free_block_groups so this prob=
lem
doesn't happen.  I will fix that.  Thanks,

Josef
--
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