[PATCH 0/2] s390/dasd: fix kernel panic with statistics

[PATCH 0/2] s390/dasd: fix kernel panic with statistics

[Stefan Haberland]

Hi Jens,

please apply the following patch for the upcomming merge window that
fixes a kernel panic that can be triggered by using dasd statistics.
Also there is a typo fix for a comment.

Thanks.

Jan Höppner (1):
  s390/dasd: protect device queue against concurrent access

Muhammad Muzammil (1):
  s390/dasd: resolve spelling mistake

 drivers/s390/block/dasd.c     | 24 +++++++++++++-----------
 drivers/s390/block/dasd_int.h |  2 +-
 2 files changed, 14 insertions(+), 12 deletions(-)

-- 
2.39.2

[PATCH 1/2] s390/dasd: resolve spelling mistake

[Stefan Haberland]

From: Muhammad Muzammil <m.muzzammilashraf@gmail.com>

resolve typing mistake from pimary to primary

Signed-off-by: Muhammad Muzammil <m.muzzammilashraf@gmail.com>
Link: https://lore.kernel.org/r/20231010043140.28416-1-m.muzzammilashraf@gmail.com
Signed-off-by: Stefan Haberland <sth@linux.ibm.com>
---
 drivers/s390/block/dasd_int.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/s390/block/dasd_int.h b/drivers/s390/block/dasd_int.h
index 8a4dbe9d7741..acdaba55b041 100644
--- a/drivers/s390/block/dasd_int.h
+++ b/drivers/s390/block/dasd_int.h
@@ -283,7 +283,7 @@ struct dasd_pprc_dev_info {
 	__u8 secondary;		/* 7       Secondary device address */
 	__u16 pprc_id;		/* 8-9     Peer-to-Peer Remote Copy ID */
 	__u8 reserved2[12];	/* 10-21   reserved */
-	__u16 prim_cu_ssid;	/* 22-23   Pimary Control Unit SSID */
+	__u16 prim_cu_ssid;	/* 22-23   Primary Control Unit SSID */
 	__u8 reserved3[12];	/* 24-35   reserved */
 	__u16 sec_cu_ssid;	/* 36-37   Secondary Control Unit SSID */
 	__u8 reserved4[90];	/* 38-127  reserved */
-- 
2.39.2

[PATCH 2/2] s390/dasd: protect device queue against concurrent access

[Stefan Haberland]

From: Jan Höppner <hoeppner@linux.ibm.com>

In dasd_profile_start() the amount of requests on the device queue are
counted. The access to the device queue is unprotected against
concurrent access. With a lot of parallel I/O, especially with alias
devices enabled, the device queue can change while dasd_profile_start()
is accessing the queue. In the worst case this leads to a kernel panic
due to incorrect pointer accesses.

Fix this by taking the device lock before accessing the queue and
counting the requests. Additionally the check for a valid profile data
pointer can be done earlier to avoid unnecessary locking in a hot path.

Cc: stable@vger.kernel.org
Fixes: 4fa52aa7a82f ("[S390] dasd: add enhanced DASD statistics interface")
Reviewed-by: Stefan Haberland <sth@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Stefan Haberland <sth@linux.ibm.com>
---
 drivers/s390/block/dasd.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index 215597f73be4..5b11ee923457 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -674,18 +674,20 @@ static void dasd_profile_start(struct dasd_block *block,
 	 * we count each request only once.
 	 */
 	device = cqr->startdev;
-	if (device->profile.data) {
-		counter = 1; /* request is not yet queued on the start device */
-		list_for_each(l, &device->ccw_queue)
-			if (++counter >= 31)
-				break;
-	}
+	if (!device->profile.data)
+		return;
+
+	spin_lock(get_ccwdev_lock(device->cdev));
+	counter = 1; /* request is not yet queued on the start device */
+	list_for_each(l, &device->ccw_queue)
+		if (++counter >= 31)
+			break;
+	spin_unlock(get_ccwdev_lock(device->cdev));
+
 	spin_lock(&device->profile.lock);
-	if (device->profile.data) {
-		device->profile.data->dasd_io_nr_req[counter]++;
-		if (rq_data_dir(req) == READ)
-			device->profile.data->dasd_read_nr_req[counter]++;
-	}
+	device->profile.data->dasd_io_nr_req[counter]++;
+	if (rq_data_dir(req) == READ)
+		device->profile.data->dasd_read_nr_req[counter]++;
 	spin_unlock(&device->profile.lock);
 }
 
-- 
2.39.2

Re: [PATCH 0/2] s390/dasd: fix kernel panic with statistics

[Stefan Haberland]

Am 25.10.23 um 15:24 schrieb Stefan Haberland:
> Hi Jens,
>
> please apply the following patch for the upcomming merge window that
> fixes a kernel panic that can be triggered by using dasd statistics.
> Also there is a typo fix for a comment.
>
> Thanks.
>
> Jan Höppner (1):
>    s390/dasd: protect device queue against concurrent access
>
> Muhammad Muzammil (1):
>    s390/dasd: resolve spelling mistake
>
>   drivers/s390/block/dasd.c     | 24 +++++++++++++-----------
>   drivers/s390/block/dasd_int.h |  2 +-
>   2 files changed, 14 insertions(+), 12 deletions(-)
>

Hi Jens,

polite ping.

Re: [PATCH 0/2] s390/dasd: fix kernel panic with statistics

[Jens Axboe]

On Wed, 25 Oct 2023 15:24:35 +0200, Stefan Haberland wrote:
> please apply the following patch for the upcomming merge window that
> fixes a kernel panic that can be triggered by using dasd statistics.
> Also there is a typo fix for a comment.
> 
> Thanks.
> 
> Jan Höppner (1):
>   s390/dasd: protect device queue against concurrent access
> 
> [...]

Applied, thanks!

[1/2] s390/dasd: resolve spelling mistake
      commit: 5029c5e4f20d8d6b41cefbde4b3eeadaec4662c6
[2/2] s390/dasd: protect device queue against concurrent access
      commit: db46cd1e0426f52999d50fa72cfa97fa39952885

Best regards,
-- 
Jens Axboe