diff mbox series

[2/7] ALSA: control: Add power state check commonly for ioctl handlers

Message ID 20210518165201.24376-3-tiwai@suse.de
State New
Headers show
Series ALSA: Prep work for PCI rescan support | expand

Commit Message

Takashi Iwai May 18, 2021, 4:51 p.m. UTC
The card power state check can be better put in the common ioctl
handler, as basically we want to prevent ioctls during the power off
state.  Although this situation won't happen normally any longer (*),
it'll be helpful for catching this for the future implementation like
the faked suspend that is needed for PCI rescan.

(*) Long long time ago, before the proper PM framework was introduced,
it was still possible to reach SNDRV_CTL_IOCTL_POWER ioctl during the
power off state.  This ioctl existed as a main control for the suspend
resume state in the past, but the feature was already dropped along
with the standard PM framework.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/control.c        | 3 +++
 sound/core/control_compat.c | 3 +++
 2 files changed, 6 insertions(+)

Comments

Jaroslav Kysela May 18, 2021, 6 p.m. UTC | #1
Dne 18. 05. 21 v 18:51 Takashi Iwai napsal(a):
> The card power state check can be better put in the common ioctl
> handler, as basically we want to prevent ioctls during the power off
> state.  Although this situation won't happen normally any longer (*),
> it'll be helpful for catching this for the future implementation like
> the faked suspend that is needed for PCI rescan.
> 
> (*) Long long time ago, before the proper PM framework was introduced,
> it was still possible to reach SNDRV_CTL_IOCTL_POWER ioctl during the
> power off state.  This ioctl existed as a main control for the suspend
> resume state in the past, but the feature was already dropped along
> with the standard PM framework.

It seems like a function dup for the 5th patch which tracks in flight the
power state. I think that we should drop this (and reshuffle patches) or
remove this in or after the 5th patch.

					Jaroslav

> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/core/control.c        | 3 +++
>  sound/core/control_compat.c | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/sound/core/control.c b/sound/core/control.c
> index 498e3701514a..c22c3fad0c64 100644
> --- a/sound/core/control.c
> +++ b/sound/core/control.c
> @@ -1772,6 +1772,9 @@ static long snd_ctl_ioctl(struct file *file, unsigned int cmd, unsigned long arg
>  	card = ctl->card;
>  	if (snd_BUG_ON(!card))
>  		return -ENXIO;
> +	err = snd_power_wait(card, SNDRV_CTL_POWER_D0);
> +	if (err < 0)
> +		return err;
>  	switch (cmd) {
>  	case SNDRV_CTL_IOCTL_PVERSION:
>  		return put_user(SNDRV_CTL_VERSION, ip) ? -EFAULT : 0;
> diff --git a/sound/core/control_compat.c b/sound/core/control_compat.c
> index 1d708aab9c98..d5b562ff237b 100644
> --- a/sound/core/control_compat.c
> +++ b/sound/core/control_compat.c
> @@ -438,6 +438,9 @@ static inline long snd_ctl_ioctl_compat(struct file *file, unsigned int cmd, uns
>  	if (snd_BUG_ON(!ctl || !ctl->card))
>  		return -ENXIO;
>  
> +	err = snd_power_wait(ctl->card, SNDRV_CTL_POWER_D0);
> +	if (err < 0)
> +		return err;
>  	switch (cmd) {
>  	case SNDRV_CTL_IOCTL_PVERSION:
>  	case SNDRV_CTL_IOCTL_CARD_INFO:
>
Takashi Iwai May 18, 2021, 7:31 p.m. UTC | #2
On Tue, 18 May 2021 20:00:46 +0200,
Jaroslav Kysela wrote:
> 
> Dne 18. 05. 21 v 18:51 Takashi Iwai napsal(a):
> > The card power state check can be better put in the common ioctl
> > handler, as basically we want to prevent ioctls during the power off
> > state.  Although this situation won't happen normally any longer (*),
> > it'll be helpful for catching this for the future implementation like
> > the faked suspend that is needed for PCI rescan.
> > 
> > (*) Long long time ago, before the proper PM framework was introduced,
> > it was still possible to reach SNDRV_CTL_IOCTL_POWER ioctl during the
> > power off state.  This ioctl existed as a main control for the suspend
> > resume state in the past, but the feature was already dropped along
> > with the standard PM framework.
> 
> It seems like a function dup for the 5th patch which tracks in flight the
> power state. I think that we should drop this (and reshuffle patches) or
> remove this in or after the 5th patch.

A good point.  With snd_power_ref_and_wait(), we achieve more
fine-grained protection, so those checks could be indeed dropped.
Will clean up later in the v2 patch.


thanks,

Takashi
diff mbox series

Patch

diff --git a/sound/core/control.c b/sound/core/control.c
index 498e3701514a..c22c3fad0c64 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -1772,6 +1772,9 @@  static long snd_ctl_ioctl(struct file *file, unsigned int cmd, unsigned long arg
 	card = ctl->card;
 	if (snd_BUG_ON(!card))
 		return -ENXIO;
+	err = snd_power_wait(card, SNDRV_CTL_POWER_D0);
+	if (err < 0)
+		return err;
 	switch (cmd) {
 	case SNDRV_CTL_IOCTL_PVERSION:
 		return put_user(SNDRV_CTL_VERSION, ip) ? -EFAULT : 0;
diff --git a/sound/core/control_compat.c b/sound/core/control_compat.c
index 1d708aab9c98..d5b562ff237b 100644
--- a/sound/core/control_compat.c
+++ b/sound/core/control_compat.c
@@ -438,6 +438,9 @@  static inline long snd_ctl_ioctl_compat(struct file *file, unsigned int cmd, uns
 	if (snd_BUG_ON(!ctl || !ctl->card))
 		return -ENXIO;
 
+	err = snd_power_wait(ctl->card, SNDRV_CTL_POWER_D0);
+	if (err < 0)
+		return err;
 	switch (cmd) {
 	case SNDRV_CTL_IOCTL_PVERSION:
 	case SNDRV_CTL_IOCTL_CARD_INFO: