Re: [PATCH v2 07/28] block: Introduce zone write plugging

Bart Van Assche
2024-03-26T17:23:51Z

On 3/25/24 20:12, Damien Le Moal wrote:
> On 3/26/24 06:53, Bart Van Assche wrote:
>> On 3/24/24 21:44, Damien Le Moal wrote:
>>> -void disk_free_zone_bitmaps(struct gendisk *disk)
>>> +static bool disk_insert_zone_wplug(struct gendisk *disk,
>>> + struct blk_zone_wplug *zwplug)
>>> +{
>>> + struct blk_zone_wplug *zwplg;
>>> + unsigned long flags;
>>> + unsigned int idx =
>>> + hash_32(zwplug->zone_no, disk->zone_wplugs_hash_bits);
>>> +
>>> + /*
>>> + * Add the new zone write plug to the hash table, but carefully as we
>>> + * are racing with other submission context, so we may already have a
>>> + * zone write plug for the same zone.
>>> + */
>>> + spin_lock_irqsave(&disk->zone_wplugs_lock, flags);
>>> + hlist_for_each_entry_rcu(zwplg, &disk->zone_wplugs_hash[idx], node) {
>>> + if (zwplg->zone_no == zwplug->zone_no) {
>>> + spin_unlock_irqrestore(&disk->zone_wplugs_lock, flags);
>>> + return false;
>>> + }
>>> + }
>>> + hlist_add_head_rcu(&zwplug->node, &disk->zone_wplugs_hash[idx]);
>>> + spin_unlock_irqrestore(&disk->zone_wplugs_lock, flags);
>>> +
>>> + return true;
>>> +}
>>
>> Since this function inserts an element into disk->zone_wplugs_hash[],
>> can it happen that another thread removes that element from the hash
>> list before this function returns?
>
> No, that cannot happen. Both insertion and deletion of plugs in the hash table
> are serialized with disk->zone_wplugs_lock. See disk_remove_zone_wplug().

I think that documenting locking assumptions with lockdep_assert_held()
would make this patch easier to review.

Thanks,

Bart.