> On 10/19/2013 12:32, Shilong Wang wrote: >> 2013/10/19, Stefan Behrens <sbehr...@giantdisaster.de>: >>> On 10/19/2013 06:17, Wang Shilong wrote: >>>> From: Wang Shilong <wangsl.f...@cn.fujitsu.com> >>>> >>>> Scrubing supers is not in a transaction context, when trying to >>>> write supers to disk, we should check if we are trying to >>>> scrub supers.Fix it. >>>> >>>> Signed-off-by: Wang Shilong <wangsl.f...@cn.fujitsu.com> >>>> --- >>>> fs/btrfs/disk-io.c | 2 ++ >>>> fs/btrfs/transaction.c | 2 ++ >>>> 2 files changed, 4 insertions(+) >>>> >>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >>>> index 419968e..0debb19 100644 >>>> --- a/fs/btrfs/disk-io.c >>>> +++ b/fs/btrfs/disk-io.c >>>> @@ -3582,7 +3582,9 @@ int btrfs_commit_super(struct btrfs_root *root) >>>> return ret; >>>> } >>>> >>>> + btrfs_scrub_pause_super(root); >>>> ret = write_ctree_super(NULL, root, 0); >>>> + btrfs_scrub_continue_super(root); >>>> return ret; >>>> } >>>> >>>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c >>>> index 277fe81..3ebcbbd 100644 >>>> --- a/fs/btrfs/transaction.c >>>> +++ b/fs/btrfs/transaction.c >>>> @@ -1892,7 +1892,9 @@ int btrfs_commit_transaction(struct >>>> btrfs_trans_handle *trans, >>>> goto cleanup_transaction; >>>> } >>>> >>>> + btrfs_scrub_pause_super(root); >>>> ret = write_ctree_super(trans, root, 0); >>>> + btrfs_scrub_continue_super(root); >>>> if (ret) { >>>> mutex_unlock(&root->fs_info->tree_log_mutex); >>>> goto cleanup_transaction; >>>> >>> >>> What kind of race do you see between writing the 4K superblock and scrub >>> checking its checksum? Or in other words, what could happen? > > > Yeah, it did not hurt. but it may output checksum mismatch. For example: > > Writing 4k superblock is not totally finished, but we are trying to scrub > > it. > > Have you ever seen this issue? > > If yes, let's find a different solution. You scrub, let's say, once a week. > Scrubbing the superblock takes, let's say, 100ms, then it's finished. This > short race doesn't justify to add such code to btrfs_commit_transaction and > btrfs_commit_super IMHO. And commiting a transaction is synchronized to scrub > already when the commit root is updated. > > If this is really an issue and these 4K disk writes and reads interfere, > let's find a better solution please.
How about this approach? We let scrub_supers in a transaction context. btrfs_join_transaction() scrub_supers btrfs_commit_transaction(). This is not elegant, but we can remove scrub_lock with supers(Notice, there is another place that have used this lock). Thanks, Wang > > -- 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