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

[Hannes Reinecke <hare@suse.de>]

On 10/21/26 11:59 AM, Damien Le Moal wrote:
> 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?
> 
Yeah, indeed. And userspace is the only caller anyway.

So you can add:

Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich