linux-kernel - Re: [PATCH] [sound] hdac-codec runtime suspended at PM:Suspend.

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: <CAJZ5v0hyk=jto8Wt0er==YmKEsgvfvZhO2wDbGOhxZaWWCNgBg@mail.gmail.com>
Date:   Mon, 12 Mar 2018 12:26:53 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Anshuman Gupta <anshuman.gupta@...el.com>
Cc:     Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        "moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." 
        <alsa-devel@...a-project.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>
Subject: Re: [PATCH] [sound] hdac-codec runtime suspended at PM:Suspend.

On Mon, Mar 12, 2018 at 12:17 PM, Anshuman Gupta
<anshuman.gupta@...el.com> wrote:
> Keep hdac-codec to be in runtime suspended while entering to suspend.
> If hdac-codec is already in runtime suspend state skip its power down
> sequence in prepare, power up sequence in complete phase.
>
> Avoid resuming hdac controller PCI device 00:1f.3 from runtime suspend
> state in case  hdac-codec already in runtime-suspend state, this is
> unnecessary and block the direct complete even for hdac controller
> PCI device 00:1f.3.
>
> This enabled direct complete path for hdac-codec and PCI device 00:1f.3.
>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@...el.com>
> ---
>  sound/soc/codecs/hdac_hdmi.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
> index f3b4f4d..810a8a6 100644
> --- a/sound/soc/codecs/hdac_hdmi.c
> +++ b/sound/soc/codecs/hdac_hdmi.c
> @@ -1852,6 +1852,8 @@ static int hdmi_codec_prepare(struct device *dev)
>         struct hdac_ext_device *edev = to_hda_ext_device(dev);
>         struct hdac_device *hdac = &edev->hdac;
>
> +       if (pm_runtime_status_suspended(dev))
> +               return 1;

This is racy in principle, because the runtime PM status can still
change after you've checked here.

But even if it isn't racy in practice, the following
pm_runtime_get_sync() becomes redundant after it, doesn't it?

>         pm_runtime_get_sync(&edev->hdac.dev);
>
>         /*
> @@ -1873,6 +1875,8 @@ static void hdmi_codec_complete(struct device *dev)
>         struct hdac_hdmi_priv *hdmi = edev->private_data;
>         struct hdac_device *hdac = &edev->hdac;
>
> +       if (pm_runtime_status_suspended(dev))
> +               return;

That, again, is somewhat fragile from the concurrency perspective.

>         /* Power up afg */
>         snd_hdac_codec_read(hdac, hdac->afg, 0, AC_VERB_SET_POWER_STATE,
>                                                         AC_PWRST_D0);
> --
> 2.7.4
>