Re: [PATCH v6 03/10] block: introduce storage element management

[Damien Le Moal <dlemoal@kernel.org>]

On 2026/10/21 11:42, Hannes Reinecke wrote:
> On 10/13/26 2:27 PM, Damien Le Moal wrote:
>> Recent SCSI (SBC) and ATA (ACS) standards define the storage element
>> depopulation feature. This feature is intended for managing hard-disks
>> heads, either combined read-write heads or pairs of read and write heads,
>> allowing to keep disks with defective heads longer in production by
>> allowing "depopulating" (removing) defective heads.
>>
>> This feature comes in two different flavors:
>>   - A destructive version which removes a head and reformats the disk at a
>>     lower capacity point, restoaring a fully functional contiguous LBA
>>     address space.
>>   - A data preserving version restricted to host-managed zoned disks, which
>>     marks the zones served by a removed head as offline or read-only.
>>
>> In preparation for using the data-preserving flavor of the depopulation
>> feature in file systems natively supporting zoned block devices, introduce
>> a set of storage element management operations and functions to define
>> generic calls into block device drivers for managing storage elements.
>>
>> The set of operations is defined with struct blk_se_ops and includes three
>> operations:
>>   - report_elements: get information on a device storage elements state
>>     reported with the new struct blk_se.
>>   - remove_element: depopulate a defective storage element
>>   - restore_elements: restore depopulated storage elements
>>
>> Each operation is called from the functions
>> bdev_report_storage_elements(), bdev_remove_storage_element() and
>> bdev_restore_storage_elements().
>>
>> Removing a healthy storage element from a device is possible and useful
>> for testing. The storage element restoration operation restore_elements
>> allows repopulating such healthy element. Repopulating defective storage
>> elements is generally not allowed by devices. The remove_element and
>> restore_elements operations are executed under the zone revalidation lock
>> so that they are serialized iand also serialized against user initiated
>> device revalidation, thus allowing to always have a consistent view of the
>> device zone conditions.
>>
>> The device drivers of zoned block devices can indicate support for the
>> storage element depopulation feature by specifying the storage element
>> management operations with the se_ops field of struct
>> block_device_operations.
>>
>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>> ---
>>   block/blk-zoned.c             | 291 +++++++++++++++++++++++++++++++++-
>>   block/blk.h                   |   5 +
>>   block/genhd.c                 |   4 +
>>   include/linux/blkdev.h        |  22 +++
>>   include/uapi/linux/blkzoned.h |  62 ++++++++
>>   5 files changed, 383 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
>> index acec8a7e648e..252d12254692 100644
>> --- a/block/blk-zoned.c
>> +++ b/block/blk-zoned.c
>> @@ -18,6 +18,7 @@
>>   #include <linux/mempool.h>
>>   #include <linux/kthread.h>
>>   #include <linux/freezer.h>
>> +#include <linux/delay.h>
>>   
>>   #include <trace/events/block.h>
>>   
>> @@ -2721,5 +2722,293 @@ int queue_zone_wplugs_show(void *data, struct seq_file *m)
>>   
>>   	return 0;
>>   }
>> -
>>   #endif
>> +
>> +static int disk_se_mgmt_in_progress(struct gendisk *disk,
>> +				     struct blk_se *elements,
>> +				     bool *in_progress)
>> +{
>> +	unsigned int i, nr_se = disk->nr_storage_elements;
>> +	int ret;
>> +
>> +	/*
>> +	 * Check if we have storage elements being removed or restored.
>> +	 */
>> +	ret = disk->fops->se_ops->report_elements(disk, elements, &nr_se);
>> +	if (ret) {
>> +		pr_err("Failed to get storage elements\n");
>> +		return ret;
>> +	}
>> +
>> +	for (i = 0; i < nr_se; i++) {
>> +		switch (elements[i].status) {
>> +		case BLK_SE_STS_REMOVE_IN_PROGRESS:
>> +		case BLK_SE_STS_RESTORE_IN_PROGRESS:
>> +			*in_progress = true;
>> +			return 0;
>> +		default:
>> +			break;
>> +		}
>> +	}
>> +
>> +	*in_progress = false;
>> +
>> +	return 0;
>> +}
>> +
>> +static int disk_check_remove_error(struct gendisk *disk,
>> +				   struct blk_se *elements,
>> +				   unsigned int element_id)
>> +{
>> +	unsigned int i;
>> +
>> +	for (i = 0; i < disk->nr_storage_elements; i++) {
>> +		if (elements[i].id == element_id &&
>> +		    elements[i].status == BLK_SE_STS_REMOVE_ERROR)
>> +			return -EIO;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int disk_wait_for_se_mgmt_completion(struct gendisk *disk,
>> +					    struct blk_se *elements,
>> +					    unsigned int element_id)
>> +{
>> +	bool in_progress;
>> +	int ret;
>> +
>> +	/*
>> +	 * Poll the storage elements heath state to detect the end of a
>> +	 * removal or restoration operation. Since we do not know how long the
>> +	 * operation may take, we cannot have a timeout on this loop. So check
>> +	 * for fatal signals so that the user can terminate this retry loop if
>> +	 * the device misbehaves.
>> +	 */
>> +	while (1) {
>> +		ret = disk_se_mgmt_in_progress(disk, elements, &in_progress);
>> +		if (ret || !in_progress)
>> +			break;
>> +
>> +		if (fatal_signal_pending(current)) {
>> +			ret = -EINTR;
>> +			break;
>> +		}
>> +
>> +		/* Not done yet: wait and retry. */
>> +		msleep(1000);
>> +	}
>> +
> I always feel a bit uncomfortable having to poll without a dedicated
> end time.
> Care to add a timeout here to avoid an infinite wait?

That's why the loop is interruptible with a signal... I fear that a timeout
could cause problem for "slow" drives, causing failures when everything is fine.
I could add a timeout of several minutes, but then who says that all drives can
depop a head in less than that timeout?



-- 
Damien Le Moal
Western Digital Research