Re: [PATCH 1/4] Btrfs: fix wrong send_in_progress accounting

[Wang Shilong <wangsl.fnst@cn.fujitsu.com>]

On 01/07/2014 11:10 AM, Wang Shilong wrote:
> Hi David,
>
> On 01/07/2014 12:30 AM, David Sterba wrote:
>> On Mon, Jan 06, 2014 at 05:25:06PM +0800, Wang Shilong wrote:
>>> Steps to reproduce:
>>>
>>>   # mkfs.btrfs -f /dev/sda8
>>>   # mount /dev/sda8 /mnt
>>>   # btrfs sub snapshot -r /mnt /mnt/snap1
>>>   # btrfs sub snapshot -r /mnt /mnt/snap2
>>>   # btrfs send /mnt/snap2 -p /mnt/snap1
>>>
>>> As @send_root will also add into clone_sources, and we should
>>> take care not to decrease its count twice.
>> Yes, send_root is appended to the clone_roots but the
>> clone_sources_to_rollback does not track this, it's not updated after
>> it's added in
>>
>>    sctx->clone_roots[sctx->clone_roots_cnt++].root = sctx->send_root;
>>
>> so send->root->send_in_progress is not modified in the loop that
>> decrements all clone_root's counters again.
>>
>> I don't see the warning that should catch negative values of the counter
>> in btrfs_root_dec_send_in_progress. Am I missing something?
> You are right here, but problem still exist.
>
> The problem is that we will sort clone sources before we decrease count,
> When sorting clone counts, we include send_root, and it will push to 
> the first
> place.....and when we try to free send_root count, it will be 
> decreased twice.
>
> Hope i am not wrong this time!
>
> Thanks Daivd to force me dig more!

My steps is a little problem, if you try this, you will find the warnig:

# btrfs sub snapshot . -r snap1
# btrfs sub snapshot . -r snap2
# btrfs send snap1 -p snap2 -f 1
# dmesg


>
>>
>>
>> david
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe 
>> linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>