RE: [PATCH v7 5/8] scsi: ufs: Fix ufshcd_hold() caused scheduling while atomic

Avri Altman
2020-02-10T08:23:35Z

> >> Fixes: f2a785ac2312 (scsi: ufshcd: Fix race between clk scaling and
> >> ungate work)
> >
> > Sorry, missed this one, if another version is needed, I will add this
> > line.
fair enough.

> >
> >>>
> >>> Signed-off-by: Can Guo <cang@codeaurora.org>
> >>> Reviewed-by: Hongwu Su <hongwus@codeaurora.org>
> >>> Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
> >>> Reviewed-by: Bean Huo <beanhuo@micron.com>
> >>> Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
> >>> ---
> >>> drivers/scsi/ufs/ufshcd.c | 5 +++++
> >>> 1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> >>> index bbc2607..e8f7f9d 100644
> >>> --- a/drivers/scsi/ufs/ufshcd.c
> >>> +++ b/drivers/scsi/ufs/ufshcd.c
> >>> @@ -1518,6 +1518,11 @@ int ufshcd_hold(struct ufs_hba *hba, bool
> >>> async)
> >>> */
> >>> if (ufshcd_can_hibern8_during_gating(hba) &&
> >>> ufshcd_is_link_hibern8(hba)) {
> >>> + if (async) {
> >>> + rc = -EAGAIN;
> >>> + hba->clk_gating.active_reqs--;
> >>> + break;
> >>> + }
> >>> spin_unlock_irqrestore(hba->host->host_lock,
> >>> flags);
> >>> flush_work(&hba->clk_gating.ungate_work);
> >>> spin_lock_irqsave(hba->host->host_lock,
> >>> flags);
> >> Since now the above code is shared in all cases,
> >> Maybe find a more economical way to pack it?
> >>
> >> Thanks,
> >> Avri
> >>
> >>
> >
> > There are only 2 of this same code pieces in ufshcd_hold() and located
> > in different cases, meanwhile there can be fall through, I don't see
> > a good way to pack it, can you suggest if you have any ideas?
> >
>
> Now, with this patch, there are 2 same code snippets located in CLKS_ON
> and REQ_CLKS_ON. If we somehow pack them, say bring in a inline func to
> pack them, we would have to tear it down later if we have to fix
> something for only one specific case by adding lines into the snippet.
> And actually this is the truth, we do have some fixes for CLKS_ON's case
> but not yet uploaded, so let's leave it as it is for now.
OK.

Thanks,
Avri