Re: [PATCH 04/15] btrfs: combine device update operations during transaction commit

Johannes Thumshirn
2019-02-05T09:21:27Z

On 04/02/2019 15:48, Nikolay Borisov wrote:
>
>
> On 4.02.19 г. 15:25 ч., Johannes Thumshirn wrote:
>> On 30/01/2019 15:50, Nikolay Borisov wrote:
>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>> index 888d72dda794..25dab68070dc 100644
>>> --- a/fs/btrfs/disk-io.c
>>> +++ b/fs/btrfs/disk-io.c
>>> @@ -4498,6 +4498,15 @@ void btrfs_cleanup_one_transaction(struct btrfs_transaction *cur_trans,
>>> ASSERT(list_empty(&cur_trans->dirty_bgs));
>>> ASSERT(list_empty(&cur_trans->io_bgs));
>>>
>>> + while (!list_empty(&cur_trans->dev_update_list)) {
>>> + struct btrfs_device *dev;
>>> +
>>> + dev = list_first_entry(&cur_trans->dev_update_list,
>>> + struct btrfs_device,
>>> + post_commit_list);
>>> + list_del_init(&dev->post_commit_list);
>>> + }
>>> +
>>
>> Why not?
>>
>> list_for_each_entry_safe(dev, next, &cur_trans->dev_update_list,
>> post_commit_list)
>> list_del_init(&dev->post_commit_list);
>
> Yep, that should work as well, given that there could be no new updates
> in the transaction at the point we are cleaning it up.

Well if there are updates added to the dev_update_list
list_for_each_entry_safe() won't work yes, but if there are concurrent
updates the dev_update_list must be protected by a lock or both of the
above constructs will break into pieces.

Johannes

--
Johannes Thumshirn SUSE Labs Filesystems
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850