Messages in this thread | | | Date | Tue, 12 Jan 2021 14:52:13 +0800 | From | Can Guo <> | Subject | Re: [PATCH 1/2] scsi: ufs: Fix a possible NULL pointer issue |
| |
On 2021-01-12 14:35, Stanley Chu wrote: > Hi Can, > > On Sat, 2021-01-02 at 05:59 -0800, Can Guo wrote: >> During system resume/suspend, hba could be NULL. In this case, do not >> touch >> eh_sem. >> >> Fixes: 88a92d6ae4fe ("scsi: ufs: Serialize eh_work with system PM >> events and async scan") >> >> Signed-off-by: Can Guo <cang@codeaurora.org> >> >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> index e221add..9829c8d 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -94,6 +94,8 @@ >> 16, 4, buf, __len, false); \ >> } while (0) >> >> +static bool early_suspend; >> + >> int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len, >> const char *prefix) >> { >> @@ -8896,8 +8898,14 @@ int ufshcd_system_suspend(struct ufs_hba *hba) >> int ret = 0; >> ktime_t start = ktime_get(); >> >> + if (!hba) { >> + early_suspend = true; >> + return 0; >> + } >> + >> down(&hba->eh_sem); >> - if (!hba || !hba->is_powered) >> + >> + if (!hba->is_powered) >> return 0; >> >> if ((ufs_get_pm_lvl_to_dev_pwr_mode(hba->spm_lvl) == >> @@ -8945,9 +8953,12 @@ int ufshcd_system_resume(struct ufs_hba *hba) >> int ret = 0; >> ktime_t start = ktime_get(); >> >> - if (!hba) { >> - up(&hba->eh_sem); >> + if (!hba) >> return -EINVAL; >> + >> + if (unlikely(early_suspend)) { >> + early_suspend = false; >> + down(&hba->eh_sem); >> } > > I guess early_suspend here is to handle the case that hba is null > during > ufshcd_system_suspend() but !null during ufshcd_system_resume(). If > yes, > would it be possible? If no, may I know what is the purpose? >
Yes, you are right. I think it is possible. platform_set_drvdata() is called in ufshcd_pltfrm_init(). Say suspend happens before platform_set_drvdata() is called, but resume comes back after platform_set_drvdata() is called. What do you think?
Thanks, Can Guo.
> Thanks a lot. > Stanley Chu > >> >> if (!hba->is_powered || pm_runtime_suspended(hba->dev))
| |