Re: [PATCH] Btrfs: fix tree corruption after multi-thread snapshots and inode cache flush

[Liu Bo <liubo2009@cn.fujitsu.com>]

On 09/29/2011 10:59 PM, Chris Mason wrote:
> Excerpts from Arne Jansen's message of 2011-09-29 04:40:30 -0400:
>> On 29.09.2011 10:36, Yan, Zheng wrote:
>>> On 09/29/2011 04:18 PM, Liu Bo wrote:
>>>> On 09/29/2011 12:25 PM, Yan, Zheng wrote:
>>>>> On 09/29/2011 10:00 AM, Liu Bo wrote:
>>>>>> The btrfs snapshotting code requires that once a root has been
>>>>>> snapshotted, we don't change it during a commit.
>>>>>>
>>>>>> But there are two cases to lead to tree corruptions:
>>>>>>
>>>>>> 1) multi-thread snapshots can commit serveral snapshots in a transaction,
>>>>>>    and this may change the src root when processing the following pending
>>>>>>    snapshots, which lead to the former snapshots corruptions;
>>>>>>
>>>>>> 2) the free inode cache was changing the roots when it root the cache,
>>>>>>    which lead to corruptions.
>>>>>>
>>>>> For the case 2, the free inode cache of newly created snapshot is invalid.
>>>>> So it's better to avoid modifying snapshotted trees.
>>>>>
>>>> For case 2, with flushing dirty inode cache during create_pending_snapshot,
>>>> we can avoid modifying snapshotted trees as your advice.
>>>>
>>>> But for case 1, I have no idea how to do the same thing, since we are not
>>>> allowed to commit per snapshot, which will make the performance terrible.
>> One snapshot per subvol per transaction is ok, but it must be possible to create
>> hundreds or thousands of snapshots for different subvols within one transaction.
>> Imagine a setup with 10000 subvols and creating one snapshot per hour.
> 
> Agreed, we need to be able to do more than one snapshot per commit.  Our
> current commits are pretty heavy, we do need to be able to batch them.
> 

Hi, Chris,

Does this "force cow" way fit your expectation?  Or we need to pick another solution,
such as building a a dependency graph among snapshots?

IMO, apart from COWing more while committing a transaction, "force cow" keeps the original
snapshot batching.

thanks,
liubo

> -chris
>