| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 19 Nov 2019 01:36:27 +0000
From: cang@...eaurora.org
To: Alim Akhtar <alim.akhtar@...il.com>
Cc: Can Guo <cang@....qualcomm.com>, asutoshd@...eaurora.org,
nguyenb@...eaurora.org, rnayak@...eaurora.org,
linux-scsi@...r.kernel.org, kernel-team@...roid.com,
saravanak@...gle.com, Mark Salyzyn <salyzyn@...gle.com>,
Alim Akhtar <alim.akhtar@...sung.com>,
Avri Altman <avri.altman@....com>,
Pedro Sousa <pedrom.sousa@...opsys.com>,
"James E.J. Bottomley" <jejb@...ux.ibm.com>,
"Martin K. Petersen" <martin.petersen@...cle.com>,
Stanley Chu <stanley.chu@...iatek.com>,
Bean Huo <beanhuo@...ron.com>,
Tomas Winkler <tomas.winkler@...el.com>,
Venkat Gopalakrishnan <venkatg@...eaurora.org>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Arnd Bergmann <arnd@...db.de>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 4/4] scsi: ufs: Complete pending requests in host reset
and restore path
On 2019-11-18 21:23, Alim Akhtar wrote:
> On Mon, Nov 18, 2019 at 9:22 AM Can Guo <cang@....qualcomm.com> wrote:
>>
>> From: Can Guo <cang@...eaurora.org>
>>
>> In UFS host reset and restore path, before probe, we stop and start
>> the
>> host controller once. After host controller is stopped, the pending
>> requests, if any, are cleared from the doorbell, but no completion IRQ
>> would be raised due to the hba is stopped.
>> These pending requests shall be completed along with the first NOP_OUT
>> command(as it is the first command which can raise a transfer
>> completion
>> IRQ) sent during probe.
>> Since the OCSs of these pending requests are not SUCCESS(because they
>> are
>> not yet literally finished), their UPIUs shall be dumped. When there
>> are
>> multiple pending requests, the UPIU dump can be overwhelming and may
>> lead
>> to stability issues because it is in atomic context.
>> Therefore, before probe, complete these pending requests right after
>> host
>> controller is stopped and silence the UPIU dump from them.
>>
>> Signed-off-by: Can Guo <cang@...eaurora.org>
>> ---
>
> Tested-by: Alim Akhtar <alim.akhtar@...sung.com>
>
> Please add all previous Ack/reviewed and tested-by tags so that we are
> aware what need to be done for this patch.
> Thanks
>
Hi Alim,
Thanks for pointing out it. I updated the patch a little bit so I think
the
prevoius tags are not valid any more.
Best Regards,
Can Guo.
>> drivers/scsi/ufs/ufshcd.c | 24 ++++++++++--------------
>> drivers/scsi/ufs/ufshcd.h | 2 ++
>> 2 files changed, 12 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 5950a7c..b92a3f4 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -4845,7 +4845,7 @@ static void ufshcd_slave_destroy(struct
>> scsi_device *sdev)
>> break;
>> } /* end of switch */
>>
>> - if (host_byte(result) != DID_OK)
>> + if ((host_byte(result) != DID_OK) && !hba->silence_err_logs)
>> ufshcd_print_trs(hba, 1 << lrbp->task_tag, true);
>> return result;
>> }
>> @@ -5404,8 +5404,8 @@ static void ufshcd_err_handler(struct
>> work_struct *work)
>>
>> /*
>> * if host reset is required then skip clearing the pending
>> - * transfers forcefully because they will automatically get
>> - * cleared after link startup.
>> + * transfers forcefully because they will get cleared during
>> + * host reset and restore
>> */
>> if (needs_reset)
>> goto skip_pending_xfer_clear;
>> @@ -6333,9 +6333,15 @@ static int ufshcd_host_reset_and_restore(struct
>> ufs_hba *hba)
>> int err;
>> unsigned long flags;
>>
>> - /* Reset the host controller */
>> + /*
>> + * Stop the host controller and complete the requests
>> + * cleared by h/w
>> + */
>> spin_lock_irqsave(hba->host->host_lock, flags);
>> ufshcd_hba_stop(hba, false);
>> + hba->silence_err_logs = true;
>> + ufshcd_complete_requests(hba);
>> + hba->silence_err_logs = false;
>> spin_unlock_irqrestore(hba->host->host_lock, flags);
>>
>> /* scale up clocks to max frequency before full
>> reinitialization */
>> @@ -6369,7 +6375,6 @@ static int ufshcd_host_reset_and_restore(struct
>> ufs_hba *hba)
>> static int ufshcd_reset_and_restore(struct ufs_hba *hba)
>> {
>> int err = 0;
>> - unsigned long flags;
>> int retries = MAX_HOST_RESET_RETRIES;
>>
>> do {
>> @@ -6379,15 +6384,6 @@ static int ufshcd_reset_and_restore(struct
>> ufs_hba *hba)
>> err = ufshcd_host_reset_and_restore(hba);
>> } while (err && --retries);
>>
>> - /*
>> - * After reset the door-bell might be cleared, complete
>> - * outstanding requests in s/w here.
>> - */
>> - spin_lock_irqsave(hba->host->host_lock, flags);
>> - ufshcd_transfer_req_compl(hba);
>> - ufshcd_tmc_handler(hba);
>> - spin_unlock_irqrestore(hba->host->host_lock, flags);
>> -
>> return err;
>> }
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>> index e0fe247..1e51034 100644
>> --- a/drivers/scsi/ufs/ufshcd.h
>> +++ b/drivers/scsi/ufs/ufshcd.h
>> @@ -513,6 +513,7 @@ struct ufs_stats {
>> * @uic_error: UFS interconnect layer error status
>> * @saved_err: sticky error mask
>> * @saved_uic_err: sticky UIC error mask
>> + * @silence_err_logs: flag to silence error logs
>> * @dev_cmd: ufs device management command information
>> * @last_dme_cmd_tstamp: time stamp of the last completed DME command
>> * @auto_bkops_enabled: to track whether bkops is enabled in device
>> @@ -670,6 +671,7 @@ struct ufs_hba {
>> u32 saved_err;
>> u32 saved_uic_err;
>> struct ufs_stats ufs_stats;
>> + bool silence_err_logs;
>>
>> /* Device management request data */
>> struct ufs_dev_cmd dev_cmd;
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum,
>> a Linux Foundation Collaborative Project
>>
Powered by blists - more mailing lists