2020-08-18 05:22:21

by Can Guo

[permalink] [raw]
Subject: [PATCH] scsi: ufs: Remove an unpaired ufshcd_scsi_unblock_requests() in err_handler()

Commit 5586dd8ea250a ("scsi: ufs: Fix a race condition between error
handler and runtime PM ops") moves the ufshcd_scsi_block_requests() inside
err_handler(), but forgets to remove the ufshcd_scsi_unblock_requests() in
the early return path. Correct the coding mistake.

Signed-off-by: Can Guo <[email protected]>

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 2b55c2e..b8441ad 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5670,7 +5670,6 @@ static void ufshcd_err_handler(struct work_struct *work)
if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
spin_unlock_irqrestore(hba->host->host_lock, flags);
- ufshcd_scsi_unblock_requests(hba);
return;
}
ufshcd_set_eh_in_progress(hba);
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


2020-08-18 05:46:55

by Asutosh Das (asd)

[permalink] [raw]
Subject: Re: [PATCH] scsi: ufs: Remove an unpaired ufshcd_scsi_unblock_requests() in err_handler()

On Mon, Aug 17 2020 at 22:21 -0700, Can Guo wrote:
>Commit 5586dd8ea250a ("scsi: ufs: Fix a race condition between error
>handler and runtime PM ops") moves the ufshcd_scsi_block_requests() inside
>err_handler(), but forgets to remove the ufshcd_scsi_unblock_requests() in
>the early return path. Correct the coding mistake.
>
>Signed-off-by: Can Guo <[email protected]>
>
Reviewed-by: Asutosh Das <[email protected]>

>diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>index 2b55c2e..b8441ad 100644
>--- a/drivers/scsi/ufs/ufshcd.c
>+++ b/drivers/scsi/ufs/ufshcd.c
>@@ -5670,7 +5670,6 @@ static void ufshcd_err_handler(struct work_struct *work)
> if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
> hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
> spin_unlock_irqrestore(hba->host->host_lock, flags);
>- ufshcd_scsi_unblock_requests(hba);
> return;
> }
> ufshcd_set_eh_in_progress(hba);
>--
>Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
>

2020-08-18 09:31:45

by hongwus

[permalink] [raw]
Subject: Re: [PATCH] scsi: ufs: Remove an unpaired ufshcd_scsi_unblock_requests() in err_handler()

On 2020-08-18 13:42, Asutosh Das wrote:
> On Mon, Aug 17 2020 at 22:21 -0700, Can Guo wrote:
>> Commit 5586dd8ea250a ("scsi: ufs: Fix a race condition between error
>> handler and runtime PM ops") moves the ufshcd_scsi_block_requests()
>> inside
>> err_handler(), but forgets to remove the
>> ufshcd_scsi_unblock_requests() in
>> the early return path. Correct the coding mistake.
>>
>> Signed-off-by: Can Guo <[email protected]>
>>
> Reviewed-by: Asutosh Das <[email protected]>
>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 2b55c2e..b8441ad 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -5670,7 +5670,6 @@ static void ufshcd_err_handler(struct
>> work_struct *work)
>> if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
>> hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
>> spin_unlock_irqrestore(hba->host->host_lock, flags);
>> - ufshcd_scsi_unblock_requests(hba);
>> return;
>> }
>> ufshcd_set_eh_in_progress(hba);
>> -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> a Linux Foundation Collaborative Project.
>>

Reviewed-by: Hongwu Su<[email protected]>

2020-08-18 14:38:35

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH] scsi: ufs: Remove an unpaired ufshcd_scsi_unblock_requests() in err_handler()

On 2020-08-17 22:20, Can Guo wrote:
> Commit 5586dd8ea250a ("scsi: ufs: Fix a race condition between error
> handler and runtime PM ops") moves the ufshcd_scsi_block_requests() inside
> err_handler(), but forgets to remove the ufshcd_scsi_unblock_requests() in
> the early return path. Correct the coding mistake.
>
> Signed-off-by: Can Guo <[email protected]>

Hi Can,

Please add a "Fixes:" tag.

Thanks,

Bart.

2020-08-20 06:27:28

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH] scsi: ufs: Remove an unpaired ufshcd_scsi_unblock_requests() in err_handler()


>
> Commit 5586dd8ea250a ("scsi: ufs: Fix a race condition between error
> handler and runtime PM ops") moves the ufshcd_scsi_block_requests() inside
> err_handler(), but forgets to remove the ufshcd_scsi_unblock_requests() in
> the early return path. Correct the coding mistake.

"fixes" tag please, for those who don't read the commit message.
Thanks,
Avri
>
> Signed-off-by: Can Guo <[email protected]>
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 2b55c2e..b8441ad 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -5670,7 +5670,6 @@ static void ufshcd_err_handler(struct work_struct
> *work)
> if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
> hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
> spin_unlock_irqrestore(hba->host->host_lock, flags);
> - ufshcd_scsi_unblock_requests(hba);
> return;
> }
> ufshcd_set_eh_in_progress(hba);
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project.

2020-08-20 06:36:50

by Can Guo

[permalink] [raw]
Subject: Re: [PATCH] scsi: ufs: Remove an unpaired ufshcd_scsi_unblock_requests() in err_handler()

On 2020-08-20 14:24, Avri Altman wrote:
>>
>> Commit 5586dd8ea250a ("scsi: ufs: Fix a race condition between error
>> handler and runtime PM ops") moves the ufshcd_scsi_block_requests()
>> inside
>> err_handler(), but forgets to remove the
>> ufshcd_scsi_unblock_requests() in
>> the early return path. Correct the coding mistake.
>
> "fixes" tag please, for those who don't read the commit message.
> Thanks,
> Avri

Already added one in the V2 of it.

Thanks,

Can Guo.

>>
>> Signed-off-by: Can Guo <[email protected]>
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 2b55c2e..b8441ad 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -5670,7 +5670,6 @@ static void ufshcd_err_handler(struct
>> work_struct
>> *work)
>> if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
>> hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
>> spin_unlock_irqrestore(hba->host->host_lock, flags);
>> - ufshcd_scsi_unblock_requests(hba);
>> return;
>> }
>> ufshcd_set_eh_in_progress(hba);
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
>> Linux
>> Foundation Collaborative Project.