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? > > - free_root_pointers(fs_info, 1); > + btrfs_stop_all_workers(fs_info); > > del_fs_roots(fs_info); > > - iput(fs_info->btree_inode); > + free_root_pointers(fs_info, 1); > > - btrfs_stop_all_workers(fs_info); > + iput(fs_info->btree_inode); > > #ifdef CONFIG_BTRFS_FS_CHECK_INTEGRITY > if (btrfs_test_opt(root, CHECK_INTEGRITY)) > -- > 1.7.7.6 > > -- > 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 Alex. -- 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
