Re: [PATCH v5 3/9] block: introduce storage element management

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] block: missing queue freeze during zoned device revalidation after element removal
--

commit b956da4759d697b5157b820cef7c5479dd817a3b
Author: Damien Le Moal <dlemoal@kernel.org>

block: introduce storage element management

This commit introduces storage element management operations for zoned block
devices. It adds functions to report, remove, and restore storage elements
to support features like element depopulation for defective heads.

> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index e7f48774ae17e..46822c7a05ecc 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c

[ ... ]

> +int bdev_remove_storage_element(struct block_device *bdev,
> +				unsigned int element_id)
> +{

[ ... ]

> +	/*
> +	 * Freeze and unfreeze the queue to flush any outstanding command.
> +	 * The caller is responsible for not queuing up more I/Os by higher
> +	 * level means.
> +	 */
> +	memflags = blk_mq_freeze_queue(disk->queue);
> +	blk_mq_unfreeze_queue(disk->queue, memflags);
> +
> +	/* Initiate the storage element removal and wait for its completion. */
> +	ret = disk->fops->se_ops->remove_element(disk, element_id);
> +	if (!ret)
> +		ret = disk_wait_for_se_mgmt_completion(disk, elements,
> +						       element_id);
> +
> +	/*
> +	 * Always revalidate zones, even in the case of an error as we do not
> +	 * know what condition the drive zones are in.
> +	 */
> +	err = disk_revalidate_zones(disk);

[Severity: High]
Could this unfreeze the queue too early?

The queue is unfrozen before disk_revalidate_zones() is called, which means
the internal zone arrays and capacities can be modified while I/O is
potentially in flight. This might allow concurrent readers in the block
layer, who rely on blk_queue_enter() for protection, to experience torn
reads or use invalid zone states.

Would it be safer to hold the queue freeze until after the zone
revalidation is complete to prevent these concurrent access issues?

[ ... ]

> +int bdev_restore_storage_elements(struct block_device *bdev)
> +{

[ ... ]

> +	/*
> +	 * Freeze and unfreeze the queue to flush any outstanding commands.
> +	 * The caller is responsible for not queuing up more I/O by higher
> +	 * level means.
> +	 */
> +	memflags = blk_mq_freeze_queue(disk->queue);
> +	blk_mq_unfreeze_queue(disk->queue, memflags);
> +
> +	/*
> +	 * Initiate the storage elements restoration and wait for its
> +	 * completion.
> +	 */
> +	ret = disk->fops->se_ops->restore_elements(disk);
> +	if (!ret)
> +		ret = disk_wait_for_se_mgmt_completion(disk, elements, 0);
> +
> +	/*
> +	 * Always revalidate zones, even in the case of an error as we do not
> +	 * know what condition the drive zones are in.
> +	 */
> +	err = disk_revalidate_zones(disk);

[Severity: High]
Does this share the same queue freeze vulnerability as the removal path?

Just like in bdev_remove_storage_element(), the queue is unfrozen prior
to calling disk_revalidate_zones(), leaving the internal state vulnerable
to concurrent I/O races during restoration.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012130121.1902634-1-dlemoal@kernel.org?part=3