> On 10/19/2013 12:32, Shilong Wang wrote:
>> 2013/10/19, Stefan Behrens <sbehrens@xxxxxxxxxxxxxxxx>:
>>> On 10/19/2013 06:17, Wang Shilong wrote:
>>>> From: Wang Shilong <wangsl.fnst@xxxxxxxxxxxxxx>
>>>>
>>>> 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.fnst@xxxxxxxxxxxxxx>
>>>> ---
>>>> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
