From: Takashi Iwai <tiwai@suse.de>
To: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
Cc: "Alexander.Deucher@amd.com" <Alexander.Deucher@amd.com>,
alsa-devel@alsa-project.org,
Sergei Miroshnichenko <s.miroshnichenko@yadro.com>,
"Christian.Koenig@amd.com" <Christian.Koenig@amd.com>
Subject: Re: Adding movable PCIe BARs support in snd_hda_intel
Date: Thu, 25 Mar 2021 07:58:21 +0100 [thread overview]
Message-ID: <s5h5z1foh2q.wl-tiwai@suse.de> (raw)
In-Reply-To: <65039cab-5561-aa9c-f270-8fa968a78f42@amd.com>
On Wed, 24 Mar 2021 22:43:24 +0100,
Andrey Grodzovsky wrote:
>
> Few comments -
>
> 1) Why we don't use snd_power_wait_and_ref in patch 3 in the common
> handler ?
> Don't we want the PCI rescan sequence to 'wait for' any in flight
> taks that might be accessing registers and not only read/write/tlv
> accesses ?
Right, we don't need to block all ioctls, but only the ones that may
access the hardware. So basically the patches 3-5 can be dropped if
we take the patch 6. The current patch was written on top of the
previous series, that's why it has both.
> 2) Possible deadlock -
> In azx_rescan_prepare - you put the card into SNDRV_CTL_POWER_D3hot
> first and then 'wait for' all in flight tasks with the refcount.
> The in flight tasks on the other hand, using snd_power_wait_and_ref,
> may have already bumped up the refcount and now 'wait for' the card
> to go into SNDRV_CTL_POWER_D0 which can't happen since PCI rescan
> waits for the refocunt to drop to 0 before proceeding.
>
> Instead of snd_power_wait_and_ref can't we just call snd_power_ref
> in common IOCTL before checking for power_state != SNDRV_CTL_POWER_D0 ?
> Or is it because you don't want to fail IOCTLs ?
No, this is the intended behavior and should work as-is because
snd_power_wait_and_ref() drops the refcount in the loop before
sleeping. The inc before the state check is a must for covering the
possible race, and ditto for changing the power_state to D3hot before
syncing.
Takashi
next prev parent reply other threads:[~2021-03-25 6:59 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-19 21:22 Adding movable PCIe BARs support in snd_hda_intel Andrey Grodzovsky
2021-03-23 11:23 ` Andrey Grodzovsky
2021-03-23 11:39 ` Takashi Iwai
2021-03-23 14:22 ` Andrey Grodzovsky
2021-03-23 14:54 ` Takashi Iwai
2021-03-23 16:11 ` Andrey Grodzovsky
2021-03-23 16:50 ` Takashi Iwai
2021-03-23 17:08 ` Andrey Grodzovsky
2021-03-23 17:29 ` Takashi Iwai
2021-03-23 18:25 ` Andrey Grodzovsky
2021-03-24 10:00 ` Takashi Iwai
2021-03-24 14:53 ` Andrey Grodzovsky
2021-03-24 15:06 ` Takashi Iwai
2021-03-24 15:43 ` Andrey Grodzovsky
2021-03-24 20:36 ` Takashi Iwai
2021-03-24 21:43 ` Andrey Grodzovsky
2021-03-25 6:58 ` Takashi Iwai [this message]
2021-03-25 16:13 ` Andrey Grodzovsky
2021-03-25 16:38 ` Takashi Iwai
2021-03-26 21:27 ` Andrey Grodzovsky
2021-03-27 8:17 ` Takashi Iwai
2021-03-29 14:47 ` Andrey Grodzovsky
2021-03-29 14:52 ` Takashi Iwai
2021-03-29 14:57 ` Andrey Grodzovsky
2021-03-24 23:36 ` [PATCH v2 1/8] ALSA: hda: Re-add dropped snd_poewr_change_state() kernel test robot
2021-03-24 23:57 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=s5h5z1foh2q.wl-tiwai@suse.de \
--to=tiwai@suse.de \
--cc=Alexander.Deucher@amd.com \
--cc=Christian.Koenig@amd.com \
--cc=alsa-devel@alsa-project.org \
--cc=andrey.grodzovsky@amd.com \
--cc=s.miroshnichenko@yadro.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox