lkml.org 
[lkml]   [2020]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 3/8] ufs: ufs-qcom: Fix a few BUGs in func ufs_qcom_dump_dbg_regs()
On 2020-07-22 22:37, Jeffrey Hugo wrote:
> On Mon, Jul 20, 2020 at 12:39 AM Can Guo <cang@codeaurora.org> wrote:
>>
>> Dumping testbus registers needs to sleep a bit intermittently as there
>> are
>> too many of them. Skip them for those contexts where sleep is not
>> allowed.
>>
>> Meanwhile, if ufs_qcom_dump_dbg_regs() calls ufs_qcom_testbus_config()
>> from
>> ufshcd_suspend/resume and/or clk gate/ungate context,
>> pm_runtime_get_sync()
>> and ufshcd_hold() will cause racing problems. Fix it by removing the
>> unnecessary calls of pm_runtime_get_sync() and ufshcd_hold().
>
> It sounds like this is two different changes which are clubbed
> together into the same patch and really should be two different
> patches.
>

Will split them and give commit msgs accordingly in next version.

>>
>> Signed-off-by: Can Guo <cang@codeaurora.org>
>> ---
>> drivers/scsi/ufs/ufs-qcom.c | 17 +++++++----------
>> 1 file changed, 7 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
>> index 2e6ddb5..3743c17 100644
>> --- a/drivers/scsi/ufs/ufs-qcom.c
>> +++ b/drivers/scsi/ufs/ufs-qcom.c
>> @@ -1604,9 +1604,6 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host
>> *host)
>> */
>> }
>> mask <<= offset;
>> -
>> - pm_runtime_get_sync(host->hba->dev);
>> - ufshcd_hold(host->hba, false);
>> ufshcd_rmwl(host->hba, TEST_BUS_SEL,
>> (u32)host->testbus.select_major << 19,
>> REG_UFS_CFG1);
>> @@ -1619,8 +1616,6 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host
>> *host)
>> * committed before returning.
>> */
>> mb();
>> - ufshcd_release(host->hba);
>> - pm_runtime_put_sync(host->hba->dev);
>>
>> return 0;
>> }
>> @@ -1658,11 +1653,13 @@ static void ufs_qcom_dump_dbg_regs(struct
>> ufs_hba *hba)
>>
>> /* sleep a bit intermittently as we are dumping too much data
>> */
>> ufs_qcom_print_hw_debug_reg_all(hba, NULL,
>> ufs_qcom_dump_regs_wrapper);
>> - udelay(1000);
>> - ufs_qcom_testbus_read(hba);
>> - udelay(1000);
>> - ufs_qcom_print_unipro_testbus(hba);
>> - udelay(1000);
>> + if (in_task()) {
>> + udelay(1000);
>> + ufs_qcom_testbus_read(hba);
>> + udelay(1000);
>> + ufs_qcom_print_unipro_testbus(hba);
>> + udelay(1000);
>> + }
>
> Did you run into a specific issue with this? udelay is not a "sleep"
> in the sense that it causes scheduling to occur, which is the problem
> with atomic contexts.

Here, ufs_qcom_print_unipro_testbus is actually causing the problem as
it has
kmalloc with flag GFP_KERNEL. Even we change the kmalloc flag to ATOMIC,
the
prints are still too heavy for atomic contexts. So we want to mute all
test bus
prints in atomic contexts. Hence the in_task() check. But apprently I
should move
the check up to have all the testbus prints inside the check. I will
modify the
change and the commit msg to tell the true story.

Thanks,

Can Guo.

\
 
 \ /
  Last update: 2020-07-23 04:14    [W:0.073 / U:0.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site