| Message ID | 1441782131.5845.73.camel@kxue-X58A-UD3R (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 09, 2015 at 03:02:11PM +0800, Ken Xue wrote: > From 844ebfcecad7ddaf7206e0474c600b0146b4ef21 Mon Sep 17 00:00:00 2001 > From: Ken Xue <Ken.Xue@amd.com> > Date: Wed, 9 Sep 2015 14:55:21 +0800 > Subject: [PATCH] SCSI:Refine the way to fix NULL pointer dereference in > runtime PM > > There was a patch about Bugzilla #101371. Which Bugzilla? I guess you should put a link or oops message of NULL pointer dereference here. Then other guys can know detail infomation of this fix. Thanks, Rui > That patch introduced a possibility that can not call > blk_{pre|post}_runtime_suspend and blk_{pre|post}_runtime_resume in > pairs. > > Signed-off-by: Ken Xue <Ken.Xue@amd.com> > --- > block/blk-core.c | 12 ++++++++++++ > drivers/scsi/scsi_pm.c | 20 ++++++++++---------- > 2 files changed, 22 insertions(+), 10 deletions(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index 60912e9..a07ab18 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -3280,6 +3280,9 @@ int blk_pre_runtime_suspend(struct request_queue *q) > { > int ret = 0; > > + if (!q->dev) > + return ret; > + > spin_lock_irq(q->queue_lock); > if (q->nr_pending) { > ret = -EBUSY; > @@ -3307,6 +3310,9 @@ EXPORT_SYMBOL(blk_pre_runtime_suspend); > */ > void blk_post_runtime_suspend(struct request_queue *q, int err) > { > + if (!q->dev) > + return; > + > spin_lock_irq(q->queue_lock); > if (!err) { > q->rpm_status = RPM_SUSPENDED; > @@ -3331,6 +3337,9 @@ EXPORT_SYMBOL(blk_post_runtime_suspend); > */ > void blk_pre_runtime_resume(struct request_queue *q) > { > + if (!q->dev) > + return; > + > spin_lock_irq(q->queue_lock); > q->rpm_status = RPM_RESUMING; > spin_unlock_irq(q->queue_lock); > @@ -3353,6 +3362,9 @@ EXPORT_SYMBOL(blk_pre_runtime_resume); > */ > void blk_post_runtime_resume(struct request_queue *q, int err) > { > + if (!q->dev) > + return; > + > spin_lock_irq(q->queue_lock); > if (!err) { > q->rpm_status = RPM_ACTIVE; > diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c > index e4b7998..459abe1 100644 > --- a/drivers/scsi/scsi_pm.c > +++ b/drivers/scsi/scsi_pm.c > @@ -219,13 +219,13 @@ static int sdev_runtime_suspend(struct device *dev) > struct scsi_device *sdev = to_scsi_device(dev); > int err = 0; > > - if (pm && pm->runtime_suspend) { > - err = blk_pre_runtime_suspend(sdev->request_queue); > - if (err) > - return err; > + err = blk_pre_runtime_suspend(sdev->request_queue); > + if (err) > + return err; > + if (pm && pm->runtime_suspend) > err = pm->runtime_suspend(dev); > - blk_post_runtime_suspend(sdev->request_queue, err); > - } > + blk_post_runtime_suspend(sdev->request_queue, err); > + > return err; > } > > @@ -248,11 +248,11 @@ static int sdev_runtime_resume(struct device *dev) > const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; > int err = 0; > > - if (pm && pm->runtime_resume) { > - blk_pre_runtime_resume(sdev->request_queue); > + blk_pre_runtime_resume(sdev->request_queue); > + if (pm && pm->runtime_resume) > err = pm->runtime_resume(dev); > - blk_post_runtime_resume(sdev->request_queue, err); > - } > + blk_post_runtime_resume(sdev->request_queue, err); > + > return err; > } > > -- > 1.9.1 > > > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2015-09-09 at 16:30 +0800, Huang Rui wrote: > On Wed, Sep 09, 2015 at 03:02:11PM +0800, Ken Xue wrote: > > From 844ebfcecad7ddaf7206e0474c600b0146b4ef21 Mon Sep 17 00:00:00 2001 > > From: Ken Xue <Ken.Xue@amd.com> > > Date: Wed, 9 Sep 2015 14:55:21 +0800 > > Subject: [PATCH] SCSI:Refine the way to fix NULL pointer dereference in > > runtime PM > > > > There was a patch about Bugzilla #101371. > > Which Bugzilla? I guess you should put a link or oops message of NULL > pointer dereference here. Then other guys can know detail infomation > of this fix. > You can find out previous patch with commit ID 49718f. You also can find log with below link https://bugzilla.kernel.org/show_bug.cgi?id=101371 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 09, 2015 at 04:40:07PM +0800, Ken Xue wrote: > On Wed, 2015-09-09 at 16:30 +0800, Huang Rui wrote: > > On Wed, Sep 09, 2015 at 03:02:11PM +0800, Ken Xue wrote: > > > From 844ebfcecad7ddaf7206e0474c600b0146b4ef21 Mon Sep 17 00:00:00 2001 > > > From: Ken Xue <Ken.Xue@amd.com> > > > Date: Wed, 9 Sep 2015 14:55:21 +0800 > > > Subject: [PATCH] SCSI:Refine the way to fix NULL pointer dereference in > > > runtime PM > > > > > > There was a patch about Bugzilla #101371. > > > > Which Bugzilla? I guess you should put a link or oops message of NULL > > pointer dereference here. Then other guys can know detail infomation > > of this fix. > > > You can find out previous patch with commit ID 49718f. > You also can find log with below link > https://bugzilla.kernel.org/show_bug.cgi?id=101371 > I see. So maybe we would better mark the commit ID. :) Alan, could you please take a look? Thanks, Rui -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 9 Sep 2015, Huang Rui wrote: > On Wed, Sep 09, 2015 at 04:40:07PM +0800, Ken Xue wrote: > > On Wed, 2015-09-09 at 16:30 +0800, Huang Rui wrote: > > > On Wed, Sep 09, 2015 at 03:02:11PM +0800, Ken Xue wrote: > > > > From 844ebfcecad7ddaf7206e0474c600b0146b4ef21 Mon Sep 17 00:00:00 2001 > > > > From: Ken Xue <Ken.Xue@amd.com> > > > > Date: Wed, 9 Sep 2015 14:55:21 +0800 > > > > Subject: [PATCH] SCSI:Refine the way to fix NULL pointer dereference in > > > > runtime PM > > > > > > > > There was a patch about Bugzilla #101371. > > > > > > Which Bugzilla? I guess you should put a link or oops message of NULL > > > pointer dereference here. Then other guys can know detail infomation > > > of this fix. > > > > > You can find out previous patch with commit ID 49718f. > > You also can find log with below link > > https://bugzilla.kernel.org/show_bug.cgi?id=101371 > > > > I see. So maybe we would better mark the commit ID. :) > > Alan, could you please take a look? Yes. Ken, your fix was good but the description was bad. The description did not explain what the problem was or how you were going to fix it. In fact, this change should be split up into two patches. The first patch should simply revert commit 49718f0fb8c9 ("SCSI: Fix NULL pointer dereference in runtime PM") and the description should explain why that commit didn't work properly for the sr driver. The second patch should make the new changes to the block core. The description should explain why the changes are needed, and it can reference Bugzilla #101371. Both patches should be tagged for the -stable kernels. When you have done all that, you can add: Acked-by: Alan Stern <stern@rowland.harvard.edu> to both patches. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/block/blk-core.c b/block/blk-core.c index 60912e9..a07ab18 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -3280,6 +3280,9 @@ int blk_pre_runtime_suspend(struct request_queue *q) { int ret = 0; + if (!q->dev) + return ret; + spin_lock_irq(q->queue_lock); if (q->nr_pending) { ret = -EBUSY; @@ -3307,6 +3310,9 @@ EXPORT_SYMBOL(blk_pre_runtime_suspend); */ void blk_post_runtime_suspend(struct request_queue *q, int err) { + if (!q->dev) + return; + spin_lock_irq(q->queue_lock); if (!err) { q->rpm_status = RPM_SUSPENDED; @@ -3331,6 +3337,9 @@ EXPORT_SYMBOL(blk_post_runtime_suspend); */ void blk_pre_runtime_resume(struct request_queue *q) { + if (!q->dev) + return; + spin_lock_irq(q->queue_lock); q->rpm_status = RPM_RESUMING; spin_unlock_irq(q->queue_lock); @@ -3353,6 +3362,9 @@ EXPORT_SYMBOL(blk_pre_runtime_resume); */ void blk_post_runtime_resume(struct request_queue *q, int err) { + if (!q->dev) + return; + spin_lock_irq(q->queue_lock); if (!err) { q->rpm_status = RPM_ACTIVE; diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c index e4b7998..459abe1 100644 --- a/drivers/scsi/scsi_pm.c +++ b/drivers/scsi/scsi_pm.c @@ -219,13 +219,13 @@ static int sdev_runtime_suspend(struct device *dev) struct scsi_device *sdev = to_scsi_device(dev); int err = 0; - if (pm && pm->runtime_suspend) { - err = blk_pre_runtime_suspend(sdev->request_queue); - if (err) - return err; + err = blk_pre_runtime_suspend(sdev->request_queue); + if (err) + return err; + if (pm && pm->runtime_suspend) err = pm->runtime_suspend(dev); - blk_post_runtime_suspend(sdev->request_queue, err); - } + blk_post_runtime_suspend(sdev->request_queue, err); + return err; } @@ -248,11 +248,11 @@ static int sdev_runtime_resume(struct device *dev) const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; int err = 0; - if (pm && pm->runtime_resume) { - blk_pre_runtime_resume(sdev->request_queue); + blk_pre_runtime_resume(sdev->request_queue); + if (pm && pm->runtime_resume) err = pm->runtime_resume(dev); - blk_post_runtime_resume(sdev->request_queue, err); - } + blk_post_runtime_resume(sdev->request_queue, err); + return err; }