Re: [PATCH v6 13/18] s390/dasd: Derive adaptive ESE fulltrack heuristic from ft_bias

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Premature `dasd_ft_bias_apply` call leaves adaptive heuristic variables uninitialized, causing FSM evaluation on every I/O
--

--- Patch [13]: [PATCH v6 13/18] s390/dasd: Derive adaptive ESE fulltrack heuristic from ft_bias ---
commit 898e7c695e4b33bd0b7466d135dcbce8e6436652
Author: Stefan Haberland <[email protected]>

s390/dasd: Derive adaptive ESE fulltrack heuristic from ft_bias

This patch derives an adaptive heuristic from the ft_bias configuration for
s390 DASD devices. It uses short probe windows to measure the NRF rate and
dynamically switches between fulltrack and plain writes, falling back and
capping the backoff interval as needed.

> diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
> index 9003e28a4ebbe..67c301e182c0f 100644
> --- a/drivers/s390/block/dasd_eckd.c
> +++ b/drivers/s390/block/dasd_eckd.c
> @@ -2162,8 +2162,7 @@ dasd_eckd_check_characteristics(struct dasd_device *device)
>  
>  	/* default ESE fulltrack write aggressiveness from the module parameter */
>  	device->ft_bias = min_t(unsigned int, full_track_bias, DASD_FT_BIAS_MAX);
> -	/* only the "always" endpoint forces fulltrack unconditionally here */
> -	device->fulltrack = (device->ft_bias >= DASD_FT_BIAS_MAX) ? 1 : 0;
> +	dasd_ft_bias_apply(device);

[Severity: High]
Does calling dasd_ft_bias_apply() happen too early here, before the volume
information is read?

Because dasd_eckd_read_vol_info() hasn't been called yet, the ESE flag is
still 0, causing dasd_ese_adaptive() to return false during initialization.
This results in dasd_ft_bias_apply() returning early and leaving heuristic
parameters like ese_heu_start_interval and ese_heu_probe_window at 0:

drivers/s390/block/dasd_int.h:dasd_ft_bias_apply() {
    ...
	if (!dasd_ese_adaptive(device)) {
		device->fulltrack = (a >= DASD_FT_BIAS_MAX) ? 1 : 0;
		device->ese_probe_state = DASD_ESE_HEU_FT1_ACTIVE;
		return;
	}
    ...
}

As a result, in dasd_ese_heuristic_tick(), the interval check will evaluate
to false on every I/O because ese_probe_interval is 0:

drivers/s390/block/dasd_eckd.c:dasd_ese_heuristic_tick() {
    ...
	if (atomic_inc_return(&basedev->ese_io_cnt) < (int)basedev->ese_probe_interval)
		return;
    ...
}

Will this completely defeat the backoff and cause the state machine to be
evaluated with atomic exchanges on every single I/O for the lifetime of the
device?

>  
>  	if (private->conf.gneq) {
>  		value = 1;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13