diff mbox series

[v2] scsi: scsi_debug: fix zone transition to full condition

Message ID 20220608011302.92061-1-damien.lemoal@opensource.wdc.com (mailing list archive)
State Accepted
Headers show
Series [v2] scsi: scsi_debug: fix zone transition to full condition | expand

Commit Message

Damien Le Moal June 8, 2022, 1:13 a.m. UTC
When a write command to a sequential write required or sequential write
preferred zone result in the zone write pointer reaching the end of the
zone, the zone condition must be set to full AND the number of
implicitly or explicitly open zones updated to have a correct accounting
for zone resources. However, the function zbc_inc_wp() only sets the
zone condition to full without updating the open zone counters,
resulting in a zone state machine breakage.

Introduce the helper function zbc_set_zone_full() and use it in
zbc_inc_wp() to correctly transition zones to the full condition.

Fixes: 0d1cf9378bd4 ("scsi: scsi_debug: Add ZBC zone commands")
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
Changes from v1:
* Simplify the patch to not modify the zbc_finish_zone() function and
  not use the CLOSED zone condition as an intermediate state in
  zbc_set_zone_full(). Cleanups to remove the use of the closed
  condition as an intermediate state will be sent later.

 drivers/scsi/scsi_debug.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

Comments

Niklas Cassel June 8, 2022, 1:52 a.m. UTC | #1
On Wed, Jun 08, 2022 at 10:13:02AM +0900, Damien Le Moal wrote:
> When a write command to a sequential write required or sequential write
> preferred zone result in the zone write pointer reaching the end of the
> zone, the zone condition must be set to full AND the number of
> implicitly or explicitly open zones updated to have a correct accounting
> for zone resources. However, the function zbc_inc_wp() only sets the
> zone condition to full without updating the open zone counters,
> resulting in a zone state machine breakage.
> 
> Introduce the helper function zbc_set_zone_full() and use it in
> zbc_inc_wp() to correctly transition zones to the full condition.
> 
> Fixes: 0d1cf9378bd4 ("scsi: scsi_debug: Add ZBC zone commands")
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---
> Changes from v1:
> * Simplify the patch to not modify the zbc_finish_zone() function and
>   not use the CLOSED zone condition as an intermediate state in
>   zbc_set_zone_full(). Cleanups to remove the use of the closed
>   condition as an intermediate state will be sent later.
> 
>  drivers/scsi/scsi_debug.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)

Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
Douglas Gilbert June 8, 2022, 5:52 p.m. UTC | #2
On 2022-06-08 03:52, Niklas Cassel wrote:
> On Wed, Jun 08, 2022 at 10:13:02AM +0900, Damien Le Moal wrote:
>> When a write command to a sequential write required or sequential write
>> preferred zone result in the zone write pointer reaching the end of the
>> zone, the zone condition must be set to full AND the number of
>> implicitly or explicitly open zones updated to have a correct accounting
>> for zone resources. However, the function zbc_inc_wp() only sets the
>> zone condition to full without updating the open zone counters,
>> resulting in a zone state machine breakage.
>>
>> Introduce the helper function zbc_set_zone_full() and use it in
>> zbc_inc_wp() to correctly transition zones to the full condition.
>>
>> Fixes: 0d1cf9378bd4 ("scsi: scsi_debug: Add ZBC zone commands")
>> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
>> ---
>> Changes from v1:
>> * Simplify the patch to not modify the zbc_finish_zone() function and
>>    not use the CLOSED zone condition as an intermediate state in
>>    zbc_set_zone_full(). Cleanups to remove the use of the closed
>>    condition as an intermediate state will be sent later.
>>
>>   drivers/scsi/scsi_debug.c | 22 ++++++++++++++++++++--
>>   1 file changed, 20 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>

Acked-by: Douglas Gilbert <dgilbert@interlog.com>
Martin K. Petersen June 10, 2022, 4:51 p.m. UTC | #3
Damien,

> Fixes: 0d1cf9378bd4 ("scsi: scsi_debug: Add ZBC zone commands")

Not sure where this SHA is from?

I fixed it up...
Martin K. Petersen June 10, 2022, 5:44 p.m. UTC | #4
On Wed, 8 Jun 2022 10:13:02 +0900, Damien Le Moal wrote:

> When a write command to a sequential write required or sequential write
> preferred zone result in the zone write pointer reaching the end of the
> zone, the zone condition must be set to full AND the number of
> implicitly or explicitly open zones updated to have a correct accounting
> for zone resources. However, the function zbc_inc_wp() only sets the
> zone condition to full without updating the open zone counters,
> resulting in a zone state machine breakage.
> 
> [...]

Applied to 5.19/scsi-fixes, thanks!

[1/1] scsi: scsi_debug: fix zone transition to full condition
      https://git.kernel.org/mkp/scsi/c/566d3c57eb52
Damien Le Moal June 12, 2022, 10:51 p.m. UTC | #5
On 6/11/22 01:51, Martin K. Petersen wrote:
> 
> Damien,
> 
>> Fixes: 0d1cf9378bd4 ("scsi: scsi_debug: Add ZBC zone commands")
> 
> Not sure where this SHA is from?
> 
> I fixed it up...

Oops. Off by one character. Missing the leading "f".
The sha is:

f0d1cf9378bd ("scsi: scsi_debug: Add ZBC zone commands")

Sorry about that.
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 1f423f723d06..b8a76b89f85a 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2826,6 +2826,24 @@  static void zbc_open_zone(struct sdebug_dev_info *devip,
 	}
 }
 
+static inline void zbc_set_zone_full(struct sdebug_dev_info *devip,
+				     struct sdeb_zone_state *zsp)
+{
+	switch (zsp->z_cond) {
+	case ZC2_IMPLICIT_OPEN:
+		devip->nr_imp_open--;
+		break;
+	case ZC3_EXPLICIT_OPEN:
+		devip->nr_exp_open--;
+		break;
+	default:
+		WARN_ONCE(true, "Invalid zone %llu condition %x\n",
+			  zsp->z_start, zsp->z_cond);
+		break;
+	}
+	zsp->z_cond = ZC5_FULL;
+}
+
 static void zbc_inc_wp(struct sdebug_dev_info *devip,
 		       unsigned long long lba, unsigned int num)
 {
@@ -2838,7 +2856,7 @@  static void zbc_inc_wp(struct sdebug_dev_info *devip,
 	if (zsp->z_type == ZBC_ZTYPE_SWR) {
 		zsp->z_wp += num;
 		if (zsp->z_wp >= zend)
-			zsp->z_cond = ZC5_FULL;
+			zbc_set_zone_full(devip, zsp);
 		return;
 	}
 
@@ -2857,7 +2875,7 @@  static void zbc_inc_wp(struct sdebug_dev_info *devip,
 			n = num;
 		}
 		if (zsp->z_wp >= zend)
-			zsp->z_cond = ZC5_FULL;
+			zbc_set_zone_full(devip, zsp);
 
 		num -= n;
 		lba += n;