On Thu, Aug 01, 2013 at 05:05:35PM +0300, Alex Lyakas wrote: > Hi Josef, > > On Thu, May 30, 2013 at 11:58 PM, Josef Bacik <jba...@fusionio.com> wrote: > > Dave reported a panic because the extent_root->commit_root was NULL in the > > caching kthread. That is because we just unset it in free_root_pointers, > > which > > is not the correct thing to do, we have to either wait for the caching > > kthread > > to complete or hold the extent_commit_sem lock so we know the thread has > > exited. > > This patch makes the kthreads all stop first and then we do our cleanup. > > This > > should fix the race. Thanks, > > > > Reported-by: David Sterba <dste...@suse.cz> > > Signed-off-by: Josef Bacik <jba...@fusionio.com> > > --- > > fs/btrfs/disk-io.c | 6 +++--- > > 1 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > > index 2b53afd..77cb566 100644 > > --- a/fs/btrfs/disk-io.c > > +++ b/fs/btrfs/disk-io.c > > @@ -3547,13 +3547,13 @@ int close_ctree(struct btrfs_root *root) > > > > btrfs_free_block_groups(fs_info); > > do you think it would be safer to stop all workers first and make sure > they are stopped, then do btrfs_free_block_groups()? I see, for > example, that btrfs_free_block_groups() checks: > if (block_group->cached == BTRFS_CACHE_STARTED) > which could be perhaps racy with other people spawning caching_threads. > > So maybe better to stop all threads (including cleaner and committer) > and then free everything? >
Well nobody should be writing anymore, so we shouldn't be starting any new caching_kthreads, we should just be cleaning up threads that are already running. Btrfs_free_block_groups() will wait on any kthreads it spawned, so we are good there. Hth, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
