Linux s390 Architecture development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Stefan Haberland" <sth@linux.ibm.com>
Cc: linux-s390@vger.kernel.org, "Vasily Gorbik" <gor@linux.ibm.com>,
	"Alexander Gordeev" <agordeev@linux.ibm.com>,
	"Heiko Carstens" <hca@linux.ibm.com>
Subject: Re: [PATCH v6 13/18] s390/dasd: Derive adaptive ESE fulltrack heuristic from ft_bias
Date: Sat, 01 Aug 2026 11:46:18 +0000	[thread overview]
Message-ID: <20260801114618.CA7231F00AC4@smtp.kernel.org> (raw)
In-Reply-To: <20260801111008.3391031-14-sth@linux.ibm.com>

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 <sth@linux.ibm.com>

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/20260801111008.3391031-1-sth@linux.ibm.com?part=13

  reply	other threads:[~2026-08-01 11:46 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-01 11:09 [PATCH v6 00/18] s390/dasd: ESE thin-provisioning performance improvements Stefan Haberland
2026-08-01 11:09 ` [PATCH v6 01/18] s390/dasd: Do not complete a failed ESE read as successful Stefan Haberland
2026-08-01 11:27   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 02/18] s390/dasd: Propagate partial completion length across ERP recovery Stefan Haberland
2026-08-01 11:21   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 03/18] s390/dasd: Guard sysfs discipline callbacks against unallocated private data Stefan Haberland
2026-08-01 11:40   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 04/18] s390/dasd: Optimize max blocks per request for track alignment Stefan Haberland
2026-08-01 11:20   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 05/18] s390/dasd: Use GFP_KERNEL in dasd_alloc_device() Stefan Haberland
2026-08-01 11:21   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 06/18] s390/dasd: Add defines for the Extended Address Volume track address Stefan Haberland
2026-08-01 11:13   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 07/18] s390/dasd: Add infrastructure for ESE full-track write Stefan Haberland
2026-08-01 11:32   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 08/18] s390/dasd: Add range-based format-track collision detection Stefan Haberland
2026-08-01 11:36   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 09/18] s390/dasd: Extend prepare_itcw() to support WRITE_FULL_TRACK Stefan Haberland
2026-08-01 11:37   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 10/18] s390/dasd: Add dasd_eckd_build_cp_tpm_writefulltrack() Stefan Haberland
2026-08-01 11:29   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 11/18] s390/dasd: Use WRITE_FULL_TRACK in ESE format handler Stefan Haberland
2026-08-01 11:39   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 12/18] s390/dasd: Add full_track_bias to control fulltrack write mode Stefan Haberland
2026-08-01 11:27   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 13/18] s390/dasd: Derive adaptive ESE fulltrack heuristic from ft_bias Stefan Haberland
2026-08-01 11:46   ` sashiko-bot [this message]
2026-08-01 11:10 ` [PATCH v6 14/18] s390/dasd: Stamp a format label into newly formatted volumes Stefan Haberland
2026-08-01 11:33   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 15/18] s390/dasd: Detect ESE volumes from the on-disk format label Stefan Haberland
2026-08-01 11:38   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 16/18] s390/dasd: Report ESE capability and format mode at device online Stefan Haberland
2026-08-01 11:45   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 17/18] s390/dasd: Re-enable discard support for ESE volumes Stefan Haberland
2026-08-01 11:45   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 18/18] s390/dasd: Read cached unit address and LSS in the CCW build path Stefan Haberland
2026-08-01 11:49   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260801114618.CA7231F00AC4@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=agordeev@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=linux-s390@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=sth@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox