Re: [PATCH v7 1/2] dm zoned: back off when reclaim cannot make progress

[Hannes Reinecke <hare@suse.de>]

On 11/3/26 11:04 PM, Kyrylo Biryuk wrote:
> When a reclaim pass cannot make progress, dmz_reclaim_work() still ends
> with dmz_schedule_reclaim(). If the target is idle, dmz_should_reclaim()
> keeps returning true, the work is queued again with no delay and the
> worker spins on the same failing pass. This patch handles two such
> cases:
> 
>    - dmz_do_reclaim() fails with -ENOSPC because no free zone is
>      available to copy the data into, for example on a target without
>      cache zones that has no free sequential zone left. The next patch
>      also makes this the expected result when an idle target with cache
>      zones has random zones but no free sequential zone.
>    - dmz_do_reclaim() finds no zone it can lock for reclaim and returns
>      -EBUSY. Since commit 90a9b8693f1b ("dm zoned: reclaim random zones
>      when idle"), dmz_should_reclaim() returns true on an idle target
>      whenever the total number of random (and cache) zones is non-zero,
>      whether or not any of them holds data. Each pass then also runs
>      dmz_check_dev(), which checks every device for media events (a TEST
>      UNIT READY on SCSI disks).
> 
> The -EBUSY loop lasts as long as the target stays idle; it ends when a
> pass finishes while the target is busy and has enough free zones. On
> three 27 TB host-managed SATA drives used as md RAID5 members (7.2.10
> with the module built out of tree, all random zones free, target idle
> for 300 seconds, kprobes attached, two other drives on the controller
> being zero-filled), each member's SCSI device counted about 108,000
> requests per second with the stock module and the system used about
> 915 s of CPU time, against about 46 s with this series. libata completes
> TEST UNIT READY in software, so these counters do not measure commands
> sent to the drives; the cost is CPU time.
> 
> When a pass fails with -ENOSPC or -EBUSY, retrying at once is unlikely
> to make progress: re-arm the idle poll instead, so that reclaim is
> retried after DMZ_IDLE_PERIOD unless a zone allocation or a "reclaim"
> message queues it earlier. On a busy target, zone allocations can
> restart reclaim immediately when free space is low enough to require
> reclaim. Re-arming also keeps the poll running after a complete idle
> drain. The poll can still stop after a pass that ends while reclaim is
> not needed; that is unchanged.
> 
> Skip the device check for these two results: they normally mean that
> there is no zone to copy into or no zone to reclaim, not that a device
> failed, and the I/O paths check the devices themselves. Use
> queue_delayed_work() so that work an allocation has already queued is
> not postponed.
> 
> A pass that fails with an I/O error, for example on an unreadable block
> of a device that stays online, is still retried at once while the
> target is idle; this patch does not change that.
> 
> Tested on 7.4-rc1 with this patch and the next one, on three emulated
> host-managed drives (scsi_debug, 1 GiB, 32 MiB zones) each under
> dm-zoned as md RAID5 members, all random zones free and the target idle
> for 300 seconds. With the stock module each member's SCSI device counted
> about 70.2 million requests and the reclaim workers used 726 s of CPU.
> With this series: 75 requests per member, 30 passes per worker (one per
> DMZ_IDLE_PERIOD) and 0.08 s of CPU. Data written during the tests read
> back with matching checksums, and no kernel warnings were logged.
> 
> Fixes: 3b1a94c88b79 ("dm zoned: drive-managed zoned block device target")
> Fixes: 90a9b8693f1b ("dm zoned: reclaim random zones when idle")
> Cc: stable@vger.kernel.org
> Signed-off-by: Kyrylo Biryuk <illofspeed@gmail.com>
> ---
>   drivers/md/dm-zoned-reclaim.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/md/dm-zoned-reclaim.c b/drivers/md/dm-zoned-reclaim.c
> index c041413c729..077a8a50197 100644
> --- a/drivers/md/dm-zoned-reclaim.c
> +++ b/drivers/md/dm-zoned-reclaim.c
> @@ -537,6 +537,16 @@ static void dmz_reclaim_work(struct work_struct *work)
>   		dmz_nr_rnd_zones(zmd, zrc->dev_idx));
>   
>   	ret = dmz_do_reclaim(zrc);
> +	if (ret == -ENOSPC || ret == -EBUSY) {
> +		/*
> +		 * No zone to copy into (-ENOSPC) or no zone to reclaim
> +		 * (-EBUSY): retrying at once cannot make progress and would
> +		 * spin. Poll again after the idle period instead, without
> +		 * postponing work that an allocation has already queued.
> +		 */
> +		queue_delayed_work(zrc->wq, &zrc->work, DMZ_IDLE_PERIOD);
> +		return;
> +	}
>   	if (ret && ret != -EINTR) {
>   		if (!dmz_check_dev(zmd))
>   			return;

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