* [RFC PATCH v1 1/2] block: bsg: resume scsi device before accessing
[not found] <cover.1611719814.git.asutoshd@codeaurora.org>
@ 2021-01-27 4:00 ` Asutosh Das
2021-01-27 7:09 ` Avri Altman
2021-02-07 2:23 ` Bart Van Assche
2021-01-27 4:00 ` [RFC PATCH v1 2/2] scsi: ufs: Fix deadlock while suspending ufs host Asutosh Das
2021-01-28 3:26 ` [RFC PATCH v2 1/2] block: bsg: resume platform device before accessing Asutosh Das
2 siblings, 2 replies; 10+ messages in thread
From: Asutosh Das @ 2021-01-27 4:00 UTC (permalink / raw)
To: cang, martin.petersen, linux-scsi
Cc: Asutosh Das, linux-arm-msm, stern, Bao D . Nguyen,
FUJITA Tomonori, Jens Axboe, open list:BLOCK LAYER, open list
Resumes the scsi device before accessing it.
Change-Id: I2929af60f2a92c89704a582fcdb285d35b429fde
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
---
block/bsg.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/block/bsg.c b/block/bsg.c
index d7bae94..f4c197f 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -306,12 +306,16 @@ static struct bsg_device *bsg_get_device(struct inode *inode, struct file *file)
static int bsg_open(struct inode *inode, struct file *file)
{
struct bsg_device *bd;
+ struct scsi_device *sd;
bd = bsg_get_device(inode, file);
if (IS_ERR(bd))
return PTR_ERR(bd);
+ sd = (struct scsi_device *) bd->queue->queuedata;
+ if (scsi_autopm_get_device(sd))
+ return -EIO;
file->private_data = bd;
return 0;
}
@@ -319,8 +323,12 @@ static int bsg_open(struct inode *inode, struct file *file)
static int bsg_release(struct inode *inode, struct file *file)
{
struct bsg_device *bd = file->private_data;
+ struct scsi_device *sd;
file->private_data = NULL;
+ sd = (struct scsi_device *) bd->queue->queuedata;
+ scsi_autopm_put_device(sd);
+
return bsg_put_device(bd);
}
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH v1 2/2] scsi: ufs: Fix deadlock while suspending ufs host
[not found] <cover.1611719814.git.asutoshd@codeaurora.org>
2021-01-27 4:00 ` [RFC PATCH v1 1/2] block: bsg: resume scsi device before accessing Asutosh Das
@ 2021-01-27 4:00 ` Asutosh Das
2021-01-28 3:26 ` [RFC PATCH v2 1/2] block: bsg: resume platform device before accessing Asutosh Das
2 siblings, 0 replies; 10+ messages in thread
From: Asutosh Das @ 2021-01-27 4:00 UTC (permalink / raw)
To: cang, martin.petersen, linux-scsi
Cc: Asutosh Das, linux-arm-msm, stern, Bao D . Nguyen, Alim Akhtar,
Avri Altman, James E.J. Bottomley, Stanley Chu, Bean Huo,
open list
During runtime-suspend of ufs host, the scsi devices are
already suspended and so are the queues associated with them.
But the ufs host sends SSU to wlun during its runtime-suspend.
During the process blk_queue_enter checks if the queue is not in
suspended state. If so, it waits for the queue to resume, and never
comes out of it.
The commit
(d55d15a33: scsi: block: Do not accept any requests while suspended)
adds the check if the queue is in suspended state in blk_queue_enter().
Fix this, by decoupling wlun scsi devices from block layer pm.
The runtime-pm for these devices would be managed by bsg and sg drivers.
Call trace:
__switch_to+0x174/0x2c4
__schedule+0x478/0x764
schedule+0x9c/0xe0
blk_queue_enter+0x158/0x228
blk_mq_alloc_request+0x40/0xa4
blk_get_request+0x2c/0x70
__scsi_execute+0x60/0x1c4
ufshcd_set_dev_pwr_mode+0x124/0x1e4
ufshcd_suspend+0x208/0x83c
ufshcd_runtime_suspend+0x40/0x154
ufshcd_pltfrm_runtime_suspend+0x14/0x20
pm_generic_runtime_suspend+0x28/0x3c
__rpm_callback+0x80/0x2a4
rpm_suspend+0x308/0x614
rpm_idle+0x158/0x228
pm_runtime_work+0x84/0xac
process_one_work+0x1f0/0x470
worker_thread+0x26c/0x4c8
kthread+0x13c/0x320
ret_from_fork+0x10/0x18
Change-Id: Id777fd52493c8b5522d1ebcad73cd30dac33e8a4
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
---
drivers/scsi/ufs/ufshcd.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 9c691e4..b7e7f81 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7217,16 +7217,6 @@ static void ufshcd_set_active_icc_lvl(struct ufs_hba *hba)
kfree(desc_buf);
}
-static inline void ufshcd_blk_pm_runtime_init(struct scsi_device *sdev)
-{
- scsi_autopm_get_device(sdev);
- blk_pm_runtime_init(sdev->request_queue, &sdev->sdev_gendev);
- if (sdev->rpm_autosuspend)
- pm_runtime_set_autosuspend_delay(&sdev->sdev_gendev,
- RPM_AUTOSUSPEND_DELAY_MS);
- scsi_autopm_put_device(sdev);
-}
-
/**
* ufshcd_scsi_add_wlus - Adds required W-LUs
* @hba: per-adapter instance
@@ -7265,7 +7255,6 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
hba->sdev_ufs_device = NULL;
goto out;
}
- ufshcd_blk_pm_runtime_init(hba->sdev_ufs_device);
scsi_device_put(hba->sdev_ufs_device);
hba->sdev_rpmb = __scsi_add_device(hba->host, 0, 0,
@@ -7274,17 +7263,14 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
ret = PTR_ERR(hba->sdev_rpmb);
goto remove_sdev_ufs_device;
}
- ufshcd_blk_pm_runtime_init(hba->sdev_rpmb);
scsi_device_put(hba->sdev_rpmb);
sdev_boot = __scsi_add_device(hba->host, 0, 0,
ufshcd_upiu_wlun_to_scsi_wlun(UFS_UPIU_BOOT_WLUN), NULL);
- if (IS_ERR(sdev_boot)) {
+ if (IS_ERR(sdev_boot))
dev_err(hba->dev, "%s: BOOT WLUN not found\n", __func__);
- } else {
- ufshcd_blk_pm_runtime_init(sdev_boot);
+ else
scsi_device_put(sdev_boot);
- }
goto out;
remove_sdev_ufs_device:
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [RFC PATCH v1 1/2] block: bsg: resume scsi device before accessing
2021-01-27 4:00 ` [RFC PATCH v1 1/2] block: bsg: resume scsi device before accessing Asutosh Das
@ 2021-01-27 7:09 ` Avri Altman
2021-01-27 7:59 ` Can Guo
2021-02-07 2:23 ` Bart Van Assche
1 sibling, 1 reply; 10+ messages in thread
From: Avri Altman @ 2021-01-27 7:09 UTC (permalink / raw)
To: Asutosh Das, cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, stern, Bao D . Nguyen, FUJITA Tomonori,
Jens Axboe, open list:BLOCK LAYER, open list
>
> Resumes the scsi device before accessing it.
>
> Change-Id: I2929af60f2a92c89704a582fcdb285d35b429fde
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
Following this patch, is it possible to revert commit 74e5e468b664d?
Thanks,
Avri
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v1 1/2] block: bsg: resume scsi device before accessing
2021-01-27 7:09 ` Avri Altman
@ 2021-01-27 7:59 ` Can Guo
2021-01-27 8:53 ` Can Guo
0 siblings, 1 reply; 10+ messages in thread
From: Can Guo @ 2021-01-27 7:59 UTC (permalink / raw)
To: Avri Altman
Cc: Asutosh Das, martin.petersen, linux-scsi, linux-arm-msm, stern,
Bao D . Nguyen, FUJITA Tomonori, Jens Axboe,
open list:BLOCK LAYER, open list
On 2021-01-27 15:09, Avri Altman wrote:
>>
>> Resumes the scsi device before accessing it.
>>
>> Change-Id: I2929af60f2a92c89704a582fcdb285d35b429fde
>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>> Signed-off-by: Can Guo <cang@codeaurora.org>
>> Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
> Following this patch, is it possible to revert commit 74e5e468b664d?
>
No, but this is a good finding... This change assumes
that the queue->queue_data is a scsi_device, which is
why we call scsi_auto_pm_get(). But for ufs_bsg's case,
queue->queue_data is a device...
Thanks,
Can Guo.
> Thanks,
> Avri
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v1 1/2] block: bsg: resume scsi device before accessing
2021-01-27 7:59 ` Can Guo
@ 2021-01-27 8:53 ` Can Guo
0 siblings, 0 replies; 10+ messages in thread
From: Can Guo @ 2021-01-27 8:53 UTC (permalink / raw)
To: Avri Altman
Cc: Asutosh Das, martin.petersen, linux-scsi, linux-arm-msm, stern,
Bao D . Nguyen, FUJITA Tomonori, Jens Axboe,
open list:BLOCK LAYER, open list
On 2021-01-27 15:59, Can Guo wrote:
> On 2021-01-27 15:09, Avri Altman wrote:
>>>
>>> Resumes the scsi device before accessing it.
>>>
>>> Change-Id: I2929af60f2a92c89704a582fcdb285d35b429fde
>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>> Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
>> Following this patch, is it possible to revert commit 74e5e468b664d?
>>
>
> No, but this is a good finding... This change assumes
> that the queue->queue_data is a scsi_device, which is
> why we call scsi_auto_pm_get(). But for ufs_bsg's case,
> queue->queue_data is a device...
>
If we call pm_runtime_get/put_sync(bcd->class_dev->parent) in
bsg_get/put_device(), commit 74e5e468b664d can be reverted.
This is just a rough idea.
> Thanks,
> Can Guo.
>
>> Thanks,
>> Avri
^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH v2 1/2] block: bsg: resume platform device before accessing
[not found] <cover.1611719814.git.asutoshd@codeaurora.org>
2021-01-27 4:00 ` [RFC PATCH v1 1/2] block: bsg: resume scsi device before accessing Asutosh Das
2021-01-27 4:00 ` [RFC PATCH v1 2/2] scsi: ufs: Fix deadlock while suspending ufs host Asutosh Das
@ 2021-01-28 3:26 ` Asutosh Das
2021-01-28 3:26 ` [RFC PATCH v2 2/2] scsi: ufs: Fix deadlock while suspending ufs host Asutosh Das
2 siblings, 1 reply; 10+ messages in thread
From: Asutosh Das @ 2021-01-28 3:26 UTC (permalink / raw)
To: cang, martin.petersen, linux-scsi
Cc: Asutosh Das, linux-arm-msm, stern, Bao D . Nguyen,
FUJITA Tomonori, Jens Axboe, open list:BLOCK LAYER, open list
It may happen that the underlying device's runtime-pm is
not controlled by block-pm. So it's possible that when
commands are sent to the device, it's suspended and may not
be resumed by blk-pm. Hence explicitly resume the parent
which is the platform device.
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
---
block/bsg.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/block/bsg.c b/block/bsg.c
index d7bae94..e9fc896 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -12,6 +12,7 @@
#include <linux/idr.h>
#include <linux/bsg.h>
#include <linux/slab.h>
+#include <linux/pm_runtime.h>
#include <scsi/scsi.h>
#include <scsi/scsi_ioctl.h>
@@ -306,12 +307,15 @@ static struct bsg_device *bsg_get_device(struct inode *inode, struct file *file)
static int bsg_open(struct inode *inode, struct file *file)
{
struct bsg_device *bd;
+ struct bsg_class_device *bcd;
bd = bsg_get_device(inode, file);
if (IS_ERR(bd))
return PTR_ERR(bd);
+ bcd = &bd->queue->bsg_dev;
+ pm_runtime_get_sync(bcd->class_dev->parent);
file->private_data = bd;
return 0;
}
@@ -319,8 +323,12 @@ static int bsg_open(struct inode *inode, struct file *file)
static int bsg_release(struct inode *inode, struct file *file)
{
struct bsg_device *bd = file->private_data;
+ struct bsg_class_device *bcd;
file->private_data = NULL;
+
+ bcd = &bd->queue->bsg_dev;
+ pm_runtime_put_sync(bcd->class_dev->parent);
return bsg_put_device(bd);
}
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH v2 2/2] scsi: ufs: Fix deadlock while suspending ufs host
2021-01-28 3:26 ` [RFC PATCH v2 1/2] block: bsg: resume platform device before accessing Asutosh Das
@ 2021-01-28 3:26 ` Asutosh Das
2021-01-28 12:21 ` Avri Altman
0 siblings, 1 reply; 10+ messages in thread
From: Asutosh Das @ 2021-01-28 3:26 UTC (permalink / raw)
To: cang, martin.petersen, linux-scsi
Cc: Asutosh Das, linux-arm-msm, stern, Bao D . Nguyen, Alim Akhtar,
Avri Altman, James E.J. Bottomley, Stanley Chu, Bean Huo,
open list
During runtime-suspend of ufs host, the scsi devices are
already suspended and so are the queues associated with them.
But the ufs host sends SSU to wlun during its runtime-suspend.
During the process blk_queue_enter checks if the queue is not in
suspended state. If so, it waits for the queue to resume, and never
comes out of it.
The commit
(d55d15a33: scsi: block: Do not accept any requests while suspended)
adds the check if the queue is in suspended state in blk_queue_enter().
Fix this, by decoupling wlun scsi devices from block layer pm.
The runtime-pm for these devices would be managed by bsg and sg drivers.
Call trace:
__switch_to+0x174/0x2c4
__schedule+0x478/0x764
schedule+0x9c/0xe0
blk_queue_enter+0x158/0x228
blk_mq_alloc_request+0x40/0xa4
blk_get_request+0x2c/0x70
__scsi_execute+0x60/0x1c4
ufshcd_set_dev_pwr_mode+0x124/0x1e4
ufshcd_suspend+0x208/0x83c
ufshcd_runtime_suspend+0x40/0x154
ufshcd_pltfrm_runtime_suspend+0x14/0x20
pm_generic_runtime_suspend+0x28/0x3c
__rpm_callback+0x80/0x2a4
rpm_suspend+0x308/0x614
rpm_idle+0x158/0x228
pm_runtime_work+0x84/0xac
process_one_work+0x1f0/0x470
worker_thread+0x26c/0x4c8
kthread+0x13c/0x320
ret_from_fork+0x10/0x18
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
---
drivers/scsi/ufs/ufshcd.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 9c691e4..b7e7f81 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7217,16 +7217,6 @@ static void ufshcd_set_active_icc_lvl(struct ufs_hba *hba)
kfree(desc_buf);
}
-static inline void ufshcd_blk_pm_runtime_init(struct scsi_device *sdev)
-{
- scsi_autopm_get_device(sdev);
- blk_pm_runtime_init(sdev->request_queue, &sdev->sdev_gendev);
- if (sdev->rpm_autosuspend)
- pm_runtime_set_autosuspend_delay(&sdev->sdev_gendev,
- RPM_AUTOSUSPEND_DELAY_MS);
- scsi_autopm_put_device(sdev);
-}
-
/**
* ufshcd_scsi_add_wlus - Adds required W-LUs
* @hba: per-adapter instance
@@ -7265,7 +7255,6 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
hba->sdev_ufs_device = NULL;
goto out;
}
- ufshcd_blk_pm_runtime_init(hba->sdev_ufs_device);
scsi_device_put(hba->sdev_ufs_device);
hba->sdev_rpmb = __scsi_add_device(hba->host, 0, 0,
@@ -7274,17 +7263,14 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
ret = PTR_ERR(hba->sdev_rpmb);
goto remove_sdev_ufs_device;
}
- ufshcd_blk_pm_runtime_init(hba->sdev_rpmb);
scsi_device_put(hba->sdev_rpmb);
sdev_boot = __scsi_add_device(hba->host, 0, 0,
ufshcd_upiu_wlun_to_scsi_wlun(UFS_UPIU_BOOT_WLUN), NULL);
- if (IS_ERR(sdev_boot)) {
+ if (IS_ERR(sdev_boot))
dev_err(hba->dev, "%s: BOOT WLUN not found\n", __func__);
- } else {
- ufshcd_blk_pm_runtime_init(sdev_boot);
+ else
scsi_device_put(sdev_boot);
- }
goto out;
remove_sdev_ufs_device:
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [RFC PATCH v2 2/2] scsi: ufs: Fix deadlock while suspending ufs host
2021-01-28 3:26 ` [RFC PATCH v2 2/2] scsi: ufs: Fix deadlock while suspending ufs host Asutosh Das
@ 2021-01-28 12:21 ` Avri Altman
2021-01-28 16:39 ` Asutosh Das
0 siblings, 1 reply; 10+ messages in thread
From: Avri Altman @ 2021-01-28 12:21 UTC (permalink / raw)
To: Asutosh Das, cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, stern, Bao D . Nguyen, Alim Akhtar,
James E.J. Bottomley, Stanley Chu, Bean Huo, open list
>
> During runtime-suspend of ufs host, the scsi devices are
> already suspended and so are the queues associated with them.
> But the ufs host sends SSU to wlun during its runtime-suspend.
Do you possible meant: "sends request-sense while clearing UAC to...."
Thanks,
Avri
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v2 2/2] scsi: ufs: Fix deadlock while suspending ufs host
2021-01-28 12:21 ` Avri Altman
@ 2021-01-28 16:39 ` Asutosh Das
0 siblings, 0 replies; 10+ messages in thread
From: Asutosh Das @ 2021-01-28 16:39 UTC (permalink / raw)
To: Avri Altman
Cc: cang, martin.petersen, linux-scsi, linux-arm-msm, stern,
Bao D . Nguyen, Alim Akhtar, James E.J. Bottomley, Stanley Chu,
Bean Huo, open list
On Thu, Jan 28 2021 at 04:21 -0800, Avri Altman wrote:
>>
>> During runtime-suspend of ufs host, the scsi devices are
>> already suspended and so are the queues associated with them.
>> But the ufs host sends SSU to wlun during its runtime-suspend.
>Do you possible meant: "sends request-sense while clearing UAC to...."
>
The idea was to show that there's a scsi command that's sent during
suspend which may deadlock.
Yes, I agree to your comment and would change the message to reflect it in
the next version.
>
>Thanks,
>Avri
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v1 1/2] block: bsg: resume scsi device before accessing
2021-01-27 4:00 ` [RFC PATCH v1 1/2] block: bsg: resume scsi device before accessing Asutosh Das
2021-01-27 7:09 ` Avri Altman
@ 2021-02-07 2:23 ` Bart Van Assche
1 sibling, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2021-02-07 2:23 UTC (permalink / raw)
To: Asutosh Das, cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, stern, Bao D . Nguyen, FUJITA Tomonori,
Jens Axboe, open list:BLOCK LAYER, open list
On 1/26/21 8:00 PM, Asutosh Das wrote:
> Resumes the scsi device before accessing it.
>
> Change-Id: I2929af60f2a92c89704a582fcdb285d35b429fde
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
No Change-Id tags in upstream patches please.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-02-07 2:24 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <cover.1611719814.git.asutoshd@codeaurora.org>
2021-01-27 4:00 ` [RFC PATCH v1 1/2] block: bsg: resume scsi device before accessing Asutosh Das
2021-01-27 7:09 ` Avri Altman
2021-01-27 7:59 ` Can Guo
2021-01-27 8:53 ` Can Guo
2021-02-07 2:23 ` Bart Van Assche
2021-01-27 4:00 ` [RFC PATCH v1 2/2] scsi: ufs: Fix deadlock while suspending ufs host Asutosh Das
2021-01-28 3:26 ` [RFC PATCH v2 1/2] block: bsg: resume platform device before accessing Asutosh Das
2021-01-28 3:26 ` [RFC PATCH v2 2/2] scsi: ufs: Fix deadlock while suspending ufs host Asutosh Das
2021-01-28 12:21 ` Avri Altman
2021-01-28 16:39 ` Asutosh Das
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).