s390/dasd: fix error recovery leading to data corruption on ESE devices

Extent Space Efficient (ESE) or thin provisioned volumes need to be
formatted on demand during usual IO processing.

The dasd_ese_needs_format function checks for error codes that signal
the non existence of a proper track format.

The check for incorrect length is to imprecise since other error cases
leading to transport of insufficient data also have this flag set.
This might lead to data corruption in certain error cases for example
during a storage server warmstart.

Fix by removing the check for incorrect length and replacing by
explicitly checking for invalid track format in transport mode.

Also remove the check for file protected since this is not a valid
ESE handling case.

Cc: stable@vger.kernel.org # 5.3+
Fixes: 5e2b17e712cf ("s390/dasd: Add dynamic formatting support for ESE volumes")
Reviewed-by: Jan Hoeppner <hoeppner@linux.ibm.com>
Signed-off-by: Stefan Haberland <sth@linux.ibm.com>
Link: https://lore.kernel.org/r/20240812125733.126431-3-sth@linux.ibm.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index 0a97cfedd7060ace133224fe718d401cb4a7fda7..42a4a996defbe1e4e39c803340b98eb05472bd69 100644 (file)
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -1601,9 +1601,15 @@ static int dasd_ese_needs_format(struct dasd_block *block, struct irb *irb)
        if (!sense)
                return 0;
 
-       return !!(sense[1] & SNS1_NO_REC_FOUND) ||
-               !!(sense[1] & SNS1_FILE_PROTECTED) ||
-               scsw_cstat(&irb->scsw) == SCHN_STAT_INCORR_LEN;
+       if (sense[1] & SNS1_NO_REC_FOUND)
+               return 1;
+
+       if ((sense[1] & SNS1_INV_TRACK_FORMAT) &&
+           scsw_is_tm(&irb->scsw) &&
+           !(sense[2] & SNS2_ENV_DATA_PRESENT))
+               return 1;
+
+       return 0;
 }
 
 static int dasd_ese_oos_cond(u8 *sense)
@@ -1624,7 +1630,7 @@ void dasd_int_handler(struct ccw_device *cdev, unsigned long intparm,
        struct dasd_device *device;
        unsigned long now;
        int nrf_suppressed = 0;
-       int fp_suppressed = 0;
+       int it_suppressed = 0;
        struct request *req;
        u8 *sense = NULL;
        int expires;
@@ -1679,8 +1685,9 @@ void dasd_int_handler(struct ccw_device *cdev, unsigned long intparm,
                 */
                sense = dasd_get_sense(irb);
                if (sense) {
-                       fp_suppressed = (sense[1] & SNS1_FILE_PROTECTED) &&
-                               test_bit(DASD_CQR_SUPPRESS_FP, &cqr->flags);
+                       it_suppressed = (sense[1] & SNS1_INV_TRACK_FORMAT) &&
+                               !(sense[2] & SNS2_ENV_DATA_PRESENT) &&
+                               test_bit(DASD_CQR_SUPPRESS_IT, &cqr->flags);
                        nrf_suppressed = (sense[1] & SNS1_NO_REC_FOUND) &&
                                test_bit(DASD_CQR_SUPPRESS_NRF, &cqr->flags);
 
@@ -1695,7 +1702,7 @@ void dasd_int_handler(struct ccw_device *cdev, unsigned long intparm,
                                return;
                        }
                }
-               if (!(fp_suppressed || nrf_suppressed))
+               if (!(it_suppressed || nrf_suppressed))
                        device->discipline->dump_sense_dbf(device, irb, "int");
 
                if (device->features & DASD_FEATURE_ERPLOG)
@@ -2459,14 +2466,17 @@ retry:
        rc = 0;
        list_for_each_entry_safe(cqr, n, ccw_queue, blocklist) {
                /*
-                * In some cases the 'File Protected' or 'Incorrect Length'
-                * error might be expected and error recovery would be
-                * unnecessary in these cases.  Check if the according suppress
-                * bit is set.
+                * In some cases certain errors might be expected and
+                * error recovery would be unnecessary in these cases.
+                * Check if the according suppress bit is set.
                 */
                sense = dasd_get_sense(&cqr->irb);
-               if (sense && sense[1] & SNS1_FILE_PROTECTED &&
-                   test_bit(DASD_CQR_SUPPRESS_FP, &cqr->flags))
+               if (sense && (sense[1] & SNS1_INV_TRACK_FORMAT) &&
+                   !(sense[2] & SNS2_ENV_DATA_PRESENT) &&
+                   test_bit(DASD_CQR_SUPPRESS_IT, &cqr->flags))
+                       continue;
+               if (sense && (sense[1] & SNS1_NO_REC_FOUND) &&
+                   test_bit(DASD_CQR_SUPPRESS_NRF, &cqr->flags))
                        continue;
                if (scsw_cstat(&cqr->irb.scsw) == 0x40 &&
                    test_bit(DASD_CQR_SUPPRESS_IL, &cqr->flags))

diff --git a/drivers/s390/block/dasd_3990_erp.c b/drivers/s390/block/dasd_3990_erp.c
index bbbacfc386f28d92ae2ab9e2091d8178328d6d55..d0aa267462c50a556f3eb718df9e29edb8fbb72b 100644 (file)
--- a/drivers/s390/block/dasd_3990_erp.c
+++ b/drivers/s390/block/dasd_3990_erp.c
@@ -1386,14 +1386,8 @@ dasd_3990_erp_file_prot(struct dasd_ccw_req * erp)
 
        struct dasd_device *device = erp->startdev;
 
-       /*
-        * In some cases the 'File Protected' error might be expected and
-        * log messages shouldn't be written then.
-        * Check if the according suppress bit is set.
-        */
-       if (!test_bit(DASD_CQR_SUPPRESS_FP, &erp->flags))
-               dev_err(&device->cdev->dev,
-                       "Accessing the DASD failed because of a hardware error\n");
+       dev_err(&device->cdev->dev,
+               "Accessing the DASD failed because of a hardware error\n");
 
        return dasd_3990_erp_cleanup(erp, DASD_CQR_FAILED);
 

diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
index 9388b5c383cab8e48e4d300fb5e41d0947f0c59d..90b106408992d0c793b02bfed7e5eec4a8d44bf9 100644 (file)
--- a/drivers/s390/block/dasd_eckd.c
+++ b/drivers/s390/block/dasd_eckd.c
@@ -2275,6 +2275,7 @@ dasd_eckd_analysis_ccw(struct dasd_device *device)
        cqr->status = DASD_CQR_FILLED;
        /* Set flags to suppress output for expected errors */
        set_bit(DASD_CQR_SUPPRESS_NRF, &cqr->flags);
+       set_bit(DASD_CQR_SUPPRESS_IT, &cqr->flags);
 
        return cqr;
 }
@@ -2556,7 +2557,6 @@ dasd_eckd_build_check_tcw(struct dasd_device *base, struct format_data_t *fdata,
        cqr->buildclk = get_tod_clock();
        cqr->status = DASD_CQR_FILLED;
        /* Set flags to suppress output for expected errors */
-       set_bit(DASD_CQR_SUPPRESS_FP, &cqr->flags);
        set_bit(DASD_CQR_SUPPRESS_IL, &cqr->flags);
 
        return cqr;
@@ -4130,8 +4130,6 @@ static struct dasd_ccw_req *dasd_eckd_build_cp_cmd_single(
 
        /* Set flags to suppress output for expected errors */
        if (dasd_eckd_is_ese(basedev)) {
-               set_bit(DASD_CQR_SUPPRESS_FP, &cqr->flags);
-               set_bit(DASD_CQR_SUPPRESS_IL, &cqr->flags);
                set_bit(DASD_CQR_SUPPRESS_NRF, &cqr->flags);
        }
 
@@ -4633,9 +4631,8 @@ static struct dasd_ccw_req *dasd_eckd_build_cp_tpm_track(
 
        /* Set flags to suppress output for expected errors */
        if (dasd_eckd_is_ese(basedev)) {
-               set_bit(DASD_CQR_SUPPRESS_FP, &cqr->flags);
-               set_bit(DASD_CQR_SUPPRESS_IL, &cqr->flags);
                set_bit(DASD_CQR_SUPPRESS_NRF, &cqr->flags);
+               set_bit(DASD_CQR_SUPPRESS_IT, &cqr->flags);
        }
 
        return cqr;
@@ -5780,36 +5777,32 @@ static void dasd_eckd_dump_sense(struct dasd_device *device,
 {
        u8 *sense = dasd_get_sense(irb);
 
-       if (scsw_is_tm(&irb->scsw)) {
-               /*
-                * In some cases the 'File Protected' or 'Incorrect Length'
-                * error might be expected and log messages shouldn't be written
-                * then. Check if the according suppress bit is set.
-                */
-               if (sense && (sense[1] & SNS1_FILE_PROTECTED) &&
-                   test_bit(DASD_CQR_SUPPRESS_FP, &req->flags))
-                       return;
-               if (scsw_cstat(&irb->scsw) == 0x40 &&
-                   test_bit(DASD_CQR_SUPPRESS_IL, &req->flags))
-                       return;
+       /*
+        * In some cases certain errors might be expected and
+        * log messages shouldn't be written then.
+        * Check if the according suppress bit is set.
+        */
+       if (sense && (sense[1] & SNS1_INV_TRACK_FORMAT) &&
+           !(sense[2] & SNS2_ENV_DATA_PRESENT) &&
+           test_bit(DASD_CQR_SUPPRESS_IT, &req->flags))
+               return;
 
-               dasd_eckd_dump_sense_tcw(device, req, irb);
-       } else {
-               /*
-                * In some cases the 'Command Reject' or 'No Record Found'
-                * error might be expected and log messages shouldn't be
-                * written then. Check if the according suppress bit is set.
-                */
-               if (sense && sense[0] & SNS0_CMD_REJECT &&
-                   test_bit(DASD_CQR_SUPPRESS_CR, &req->flags))
-                       return;
+       if (sense && sense[0] & SNS0_CMD_REJECT &&
+           test_bit(DASD_CQR_SUPPRESS_CR, &req->flags))
+               return;
 
-               if (sense && sense[1] & SNS1_NO_REC_FOUND &&
-                   test_bit(DASD_CQR_SUPPRESS_NRF, &req->flags))
-                       return;
+       if (sense && sense[1] & SNS1_NO_REC_FOUND &&
+           test_bit(DASD_CQR_SUPPRESS_NRF, &req->flags))
+               return;
 
+       if (scsw_cstat(&irb->scsw) == 0x40 &&
+           test_bit(DASD_CQR_SUPPRESS_IL, &req->flags))
+               return;
+
+       if (scsw_is_tm(&irb->scsw))
+               dasd_eckd_dump_sense_tcw(device, req, irb);
+       else
                dasd_eckd_dump_sense_ccw(device, req, irb);
-       }
 }
 
 static int dasd_eckd_reload_device(struct dasd_device *device)

diff --git a/drivers/s390/block/dasd_int.h b/drivers/s390/block/dasd_int.h
index e5f40536b4254027c3c181a6efb29e318fa72f7a..81cfb5c89681bc9612acac0f333b8e6d7e4896a8 100644 (file)
--- a/drivers/s390/block/dasd_int.h
+++ b/drivers/s390/block/dasd_int.h
@@ -196,7 +196,7 @@ struct dasd_ccw_req {
  * The following flags are used to suppress output of certain errors.
  */
 #define DASD_CQR_SUPPRESS_NRF  4       /* Suppress 'No Record Found' error */
-#define DASD_CQR_SUPPRESS_FP   5       /* Suppress 'File Protected' error*/
+#define DASD_CQR_SUPPRESS_IT   5       /* Suppress 'Invalid Track' error*/
 #define DASD_CQR_SUPPRESS_IL   6       /* Suppress 'Incorrect Length' error */
 #define DASD_CQR_SUPPRESS_CR   7       /* Suppress 'Command Reject' error */