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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6d636c36-ce05-4825-b916-b97484c41c5b@quicinc.com>
Date: Tue, 11 Jun 2024 19:02:43 +0800
From: Ziqi Chen <quic_ziqichen@...cinc.com>
To: Bart Van Assche <bvanassche@....org>, <quic_cang@...cinc.com>,
        <mani@...nel.org>, <beanhuo@...ron.com>, <avri.altman@....com>,
        <junwoo80.lee@...sung.com>, <martin.petersen@...cle.com>,
        <quic_nguyenb@...cinc.com>, <quic_nitirawa@...cinc.com>,
        <quic_rampraka@...cinc.com>
CC: <linux-scsi@...r.kernel.org>, Alim Akhtar <alim.akhtar@...sung.com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        Peter Wang
	<peter.wang@...iatek.com>,
        Manivannan Sadhasivam
	<manivannan.sadhasivam@...aro.org>,
        Maramaina Naresh
	<quic_mnaresh@...cinc.com>,
        Asutosh Das <quic_asutoshd@...cinc.com>,
        "open
 list" <linux-kernel@...r.kernel.org>,
        <quic_richardp@...cinc.com>
Subject: Re: [PATCH] scsi: ufs: core: quiesce request queues before check
 pending cmds



On 6/7/2024 8:33 PM, Bart Van Assche wrote:
> On 6/7/24 04:06, Ziqi Chen wrote:
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index 21429ee..1afa862 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -1392,7 +1392,7 @@ static int ufshcd_clock_scaling_prepare(struct 
>> ufs_hba *hba, u64 timeout_us)
>>        * make sure that there are no outstanding requests when
>>        * clock scaling is in progress
>>        */
>> -    ufshcd_scsi_block_requests(hba);
>> +    blk_mq_quiesce_tagset(&hba->host->tag_set);
>>       mutex_lock(&hba->wb_mutex);
>>       down_write(&hba->clk_scaling_lock);
>> @@ -1401,7 +1401,7 @@ static int ufshcd_clock_scaling_prepare(struct 
>> ufs_hba *hba, u64 timeout_us)
>>           ret = -EBUSY;
>>           up_write(&hba->clk_scaling_lock);
>>           mutex_unlock(&hba->wb_mutex);
>> -        ufshcd_scsi_unblock_requests(hba);
>> +        blk_mq_unquiesce_tagset(&hba->host->tag_set);
>>           goto out;
>>       }
>> @@ -1422,7 +1422,7 @@ static void 
>> ufshcd_clock_scaling_unprepare(struct ufs_hba *hba, int err, bool sc
>>       mutex_unlock(&hba->wb_mutex);
>> -    ufshcd_scsi_unblock_requests(hba);
>> +    blk_mq_unquiesce_tagset(&hba->host->tag_set);
>>       ufshcd_release(hba);
>>   }
> 
> Why to replace only those ufshcd_scsi_block_requests() /
> ufshcd_scsi_unblock_requests() calls? I don't think that it is ever safe to
> use these functions instead of  blk_mq_quiesce_tagset() /
> blk_mq_unquiesce_tagset(). Please replace all 
> ufshcd_scsi_block_requests() /
> ufshcd_scsi_unblock_requests() calls and remove the
> ufshcd_scsi_*block_requests() functions.

Hi Bart,

Thank you for the review.

This issue was not easy to debug, it took us more than 3 months to 
narrow down to the root cause. Our mutual customers and internal test 
teams had to pull in quite amount of resources to help narrow down the 
issue and test the fix. It is a key change to unblock customers from 
commercializing Android15, so we are pushed to upstream this fix ASAP.

As for removing the rest calls to ufshcd_scsi_block_requests() and 
ufshcd_scsi_unblock_requests(), I think you are right, but I am not 
quite sure because we haven't seen issue reported w.r.t those spots. If 
possible, we can co-work on this sometime later.

Hope you can understand.

Thanks,

Ziqi

> 
> Thanks,
> 
> Bart.
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ