linux-kernel - Re: [PATCH v3 1/2] scsi: ufs: core: Introduce a new clock

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: <2d9aa1f9ed2e63d29dc2a7745fdd4f6db45d8db0.camel@gmail.com>
Date: Tue, 12 Nov 2024 00:16:58 +0100
From: Bean Huo <huobean@...il.com>
To: Avri Altman <avri.altman@....com>, "Martin K . Petersen"
	 <martin.petersen@...cle.com>, beanhuo@...ron.com
Cc: linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org, Bart Van
 Assche <bvanassche@....org>
Subject: Re: [PATCH v3 1/2] scsi: ufs: core: Introduce a new clock_gating
 lock

On Tue, 2024-11-05 at 13:25 +0200, Avri Altman wrote:
> -       spin_lock_irqsave(hba->host->host_lock, flags);
> -       /*
> -        * In case you are here to cancel this work the gating state
> -        * would be marked as REQ_CLKS_ON. In this case save time by
> -        * skipping the gating work and exit after changing the clock
> -        * state to CLKS_ON.
> -        */
> -       if (hba->clk_gating.is_suspended ||
> -               (hba->clk_gating.state != REQ_CLKS_OFF)) {
> -               hba->clk_gating.state = CLKS_ON;
> -               trace_ufshcd_clk_gating(dev_name(hba->dev),
> -                                       hba->clk_gating.state);
> -               goto rel_lock;
> +       scoped_guard(spinlock_irqsave, &hba->clk_gating.lock)
> +       {
> +               /*
> +                * In case you are here to cancel this work the
> gating state
> +                * would be marked as REQ_CLKS_ON. In this case save
> time by
> +                * skipping the gating work and exit after changing
> the clock
> +                * state to CLKS_ON.
> +                */
> +               if (hba->clk_gating.is_suspended ||
> +                   hba->clk_gating.state != REQ_CLKS_OFF) {
> +                       hba->clk_gating.state = CLKS_ON;
> +                       trace_ufshcd_clk_gating(dev_name(hba->dev),
> +                                               hba-
> >clk_gating.state);
> +                       return;
> +               }
> +               if (ufshcd_is_ufs_dev_busy(hba) ||
> +                   hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL)
> +                       return;
>         }

I'm wondering if it would be safe to replace host_lock with gating.lock
or scaling.lock. For instance, in above context, ufshcd_state needs to
be checked, but it's currently serialized by host_lock.

King regards,
Bean