On 10/28/2016 05:05 PM, Bart Van Assche wrote:
On 10/28/2016 01:21 AM, [email protected] wrote:
From: "tang.junhui" <[email protected]>
scsi_device_put() is called when the conditions pg->rtpg_sdev!=NULL
and queue_delayed_work() failure satisfied, actually it should be not
to call because scsi_device_get() is not called previous in this scene.
Signed-off-by: tang.junhui <[email protected]>
---
drivers/scsi/device_handler/scsi_dh_alua.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
b/drivers/scsi/device_handler/scsi_dh_alua.c
index 241829e..78081df 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -866,6 +866,7 @@ static void alua_rtpg_queue(struct alua_port_group *pg,
struct alua_queue_data *qdata, bool force)
{
int start_queue = 0;
+ int sdev_geted = 0;
unsigned long flags;
struct workqueue_struct *alua_wq = kaluad_wq;
@@ -884,6 +885,7 @@ static void alua_rtpg_queue(struct alua_port_group *pg,
kref_get(&pg->kref);
pg->rtpg_sdev = sdev;
scsi_device_get(sdev);
+ sdev_geted = 1;
start_queue = 1;
} else if (!(pg->flags & ALUA_PG_RUN_RTPG) && force) {
pg->flags |= ALUA_PG_RUN_RTPG;
@@ -901,7 +903,8 @@ static void alua_rtpg_queue(struct alua_port_group *pg,
if (start_queue &&
!queue_delayed_work(alua_wq, &pg->rtpg_work,
msecs_to_jiffies(ALUA_RTPG_DELAY_MSECS))) {
- scsi_device_put(sdev);
+ if (sdev_geted)
+ scsi_device_put(sdev);
kref_put(&pg->kref, release_port_group);
}
}
Hello Tang Junhui,
Personally I prefer the patch below because that patch does not introduce
a new variable. This is a patch I started testing four weeks ago and it has
survived all my ALUA tests so far. Please note that I do not know whether
my tests hit the !(pg->flags & ALUA_PG_RUNNING) code path.
From: Bart Van Assche <[email protected]>
Date: Thu, 29 Sep 2016 10:06:51 -0700
Subject: [PATCH] scsi_dh_alua: Fix a reference counting bug
Signed-off-by: Bart Van Assche <[email protected]>
Cc: <[email protected]>
---
drivers/scsi/device_handler/scsi_dh_alua.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
b/drivers/scsi/device_handler/scsi_dh_alua.c
index 241829e..23835f7 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -890,6 +890,7 @@ static void alua_rtpg_queue(struct alua_port_group *pg,
/* Do not queue if the worker is already running */
if (!(pg->flags & ALUA_PG_RUNNING)) {
kref_get(&pg->kref);
+ sdev = NULL;
start_queue = 1;
}
}
@@ -901,7 +902,8 @@ static void alua_rtpg_queue(struct alua_port_group *pg,
if (start_queue &&
!queue_delayed_work(alua_wq, &pg->rtpg_work,
msecs_to_jiffies(ALUA_RTPG_DELAY_MSECS))) {
- scsi_device_put(sdev);
+ if (sdev)
+ scsi_device_put(sdev);
kref_put(&pg->kref, release_port_group);
}
}
Yes, this looks far better.
Reviewed-by: Hannes Reinecke <[email protected]>
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
[email protected] +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
