[PATCH v3 08/30] block: Introduce zone write plugging

[Bart Van Assche]

On 3/27/24 5:43 PM, Damien Le Moal wrote:
> +	/*
> +	 * Initialize the zoen write plug with an extra reference so that
> +	 * it is not freed when the zone write plug becomes idle without
> +	 * the zone being full.
> +	 */

zoen -> zone

> +static void disk_zone_wplugs_work(struct work_struct *work)
> +{
> +	struct gendisk *disk =
> +		container_of(work, struct gendisk, zone_wplugs_work);
> +	struct blk_zone_wplug *zwplug;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&disk->zone_wplugs_lock, flags);
> +
> +	while (!list_empty(&disk->zone_wplugs_err_list)) {
> +		zwplug = list_first_entry(&disk->zone_wplugs_err_list,
> +					  struct blk_zone_wplug, link);
> +		list_del_init(&zwplug->link);
> +		blk_get_zone_wplug(zwplug);
> +		spin_unlock_irqrestore(&disk->zone_wplugs_lock, flags);
> +
> +		disk_zone_wplug_handle_error(disk, zwplug);
> +		disk_put_zone_wplug(zwplug);
> +
> +		spin_lock_irqsave(&disk->zone_wplugs_lock, flags);
> +	}
> +
> +	spin_unlock_irqrestore(&disk->zone_wplugs_lock, flags);
> +}

What is the maximum number of iterations the above while loop can 
perform? I'm wondering whether a cond_resched() call should be added.

> +
> +	/* Wait for the zone write plugs to be RCU-freed. */
> +	rcu_barrier();
> +}

It is not clear to me why the above rcu_barrier() call is necessary? I'm
not aware of any other kernel code where kfree_rcu() is followed by an
rcu_barrier() call.

> +static int disk_alloc_zone_resources(struct gendisk *disk,
> +				     unsigned int max_nr_zwplugs)
> +{
> +	unsigned int i;
> +
> +	disk->zone_wplugs_hash_bits =
> +		min(ilog2(max_nr_zwplugs) + 1, BLK_ZONE_MAX_WPLUG_HASH_BITS);

If max_nr_zwplugs is a power of two, the above formula will result in a
hash table with a size that is twice the size of max_nr_zwplugs.
Shouldn't ilog2(max_nr_zwplugs) + 1 be changed into
ilog2(roundup_pow_of_two(max_nr_zwplugs))?

Otherwise this patch looks fine to me.

Thanks,

Bart.