All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11] ALSA: PCM state reference optimization
@ 2022-09-26 13:55 Takashi Iwai
  2022-09-26 13:55 ` [PATCH 01/11] ALSA: pcm: Avoid reference to status->state Takashi Iwai
                   ` (12 more replies)
  0 siblings, 13 replies; 20+ messages in thread
From: Takashi Iwai @ 2022-09-26 13:55 UTC (permalink / raw)
  To: alsa-devel
  Cc: Felipe Balbi, Cezary Rojewski, Kai Vehmanen, Takashi Iwai,
	Greg Kroah-Hartman, Peter Ujfalusi, Pierre-Louis Bossart,
	Ranjani Sridharan, Liam Girdwood, Mark Brown, Bard Liao

Hi,

this is a patch set for simplifying the reference to the current PCM
state by having the local copy in runtime instead of relying on
runtime->status indirection.  This also hardens against the attack by
modifying the mmapped status record.

The first patch does the basic job in the core PCM side, and the
second patch flips the PCM status mmap to read-only for hardening,
while the remaining patches are for drivers to follow the core
change.

The conversions are straightforward.  In most places, it's just
replacing runtime->status->state with runtime->state.


Takashi

===

Takashi Iwai (11):
  ALSA: pcm: Avoid reference to status->state
  ALSA: pcm: Make mmap status read-only
  ALSA: aloop: Replace runtime->status->state reference to
    runtime->state
  ALSA: firewire: Replace runtime->status->state reference to
    runtime->state
  ALSA: hda: Replace runtime->status->state reference to runtime->state
  ALSA: asihpi: Replace runtime->status->state reference to
    runtime->state
  ALSA: usb-audio: Replace runtime->status->state reference to
    runtime->state
  ALSA: usx2y: Replace runtime->status->state reference to
    runtime->state
  ASoC: intel: Replace runtime->status->state reference to
    runtime->state
  ASoC: sh: Replace runtime->status->state reference to runtime->state
  usb: gadget: Replace runtime->status->state reference to
    runtime->state

 drivers/usb/gadget/function/u_uac1_legacy.c |   4 +-
 include/sound/pcm.h                         |  20 ++-
 sound/core/oss/pcm_oss.c                    |  42 +++----
 sound/core/pcm.c                            |   9 +-
 sound/core/pcm_compat.c                     |   4 +-
 sound/core/pcm_lib.c                        |  16 +--
 sound/core/pcm_native.c                     | 128 ++++++++++----------
 sound/drivers/aloop.c                       |   4 +-
 sound/firewire/bebob/bebob_pcm.c            |   4 +-
 sound/firewire/dice/dice-pcm.c              |   4 +-
 sound/firewire/digi00x/digi00x-pcm.c        |   4 +-
 sound/firewire/fireface/ff-pcm.c            |   4 +-
 sound/firewire/fireworks/fireworks_pcm.c    |   4 +-
 sound/firewire/motu/motu-pcm.c              |   4 +-
 sound/firewire/oxfw/oxfw-pcm.c              |   8 +-
 sound/firewire/tascam/tascam-pcm.c          |   4 +-
 sound/hda/hdmi_chmap.c                      |   2 +-
 sound/pci/asihpi/asihpi.c                   |   2 +-
 sound/soc/intel/skylake/skl-pcm.c           |   4 +-
 sound/soc/sh/rz-ssi.c                       |   2 +-
 sound/usb/pcm.c                             |   4 +-
 sound/usb/usx2y/usbusx2yaudio.c             |   3 +-
 sound/usb/usx2y/usx2yhwdeppcm.c             |   3 +-
 23 files changed, 150 insertions(+), 133 deletions(-)

===

Cc: Bard Liao <yung-chuan.liao@linux.intel.com>
Cc: Cezary Rojewski <cezary.rojewski@intel.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Cc: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Cc: Takashi Sakamoto <o-takashi@sakamocchi.jp>

-- 
2.35.3


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 01/11] ALSA: pcm: Avoid reference to status->state
  2022-09-26 13:55 [PATCH 00/11] ALSA: PCM state reference optimization Takashi Iwai
@ 2022-09-26 13:55 ` Takashi Iwai
  2022-09-26 13:55 ` [PATCH 02/11] ALSA: pcm: Make mmap status read-only Takashi Iwai
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2022-09-26 13:55 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai

In the PCM core and driver code, there are lots place referring to the
current PCM state via runtime->status->state.  This patch introduced a
local PCM state in runtime itself and replaces those references with
runtime->state.  It has improvements in two aspects:

- The reduction of a indirect access leads to more code optimization

- It avoids a possible (unexpected) modification of the state via mmap
  of the status record

The status->state is updated together with runtime->state, so that
user-space can still read the current state via mmap like before,
too.

This patch touches only the ALSA core code.  The changes in each
driver will follow in later patches.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/pcm.h      |  20 +++++-
 sound/core/oss/pcm_oss.c |  42 ++++++-------
 sound/core/pcm.c         |   9 +--
 sound/core/pcm_compat.c  |   4 +-
 sound/core/pcm_lib.c     |  16 ++---
 sound/core/pcm_native.c  | 127 ++++++++++++++++++++-------------------
 6 files changed, 118 insertions(+), 100 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 8c48a5bce88c..7b1a022910e8 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -346,6 +346,8 @@ static inline void snd_pcm_pack_audio_tstamp_report(__u32 *data, __u32 *accuracy
 
 struct snd_pcm_runtime {
 	/* -- Status -- */
+	snd_pcm_state_t state;		/* stream state */
+	snd_pcm_state_t suspended_state; /* suspended stream state */
 	struct snd_pcm_substream *trigger_master;
 	struct timespec64 trigger_tstamp;	/* trigger timestamp */
 	bool trigger_tstamp_latched;     /* trigger timestamp latched in low-level driver/hardware */
@@ -678,11 +680,25 @@ void snd_pcm_stream_unlock_irqrestore(struct snd_pcm_substream *substream,
  */
 static inline int snd_pcm_running(struct snd_pcm_substream *substream)
 {
-	return (substream->runtime->status->state == SNDRV_PCM_STATE_RUNNING ||
-		(substream->runtime->status->state == SNDRV_PCM_STATE_DRAINING &&
+	return (substream->runtime->state == SNDRV_PCM_STATE_RUNNING ||
+		(substream->runtime->state == SNDRV_PCM_STATE_DRAINING &&
 		 substream->stream == SNDRV_PCM_STREAM_PLAYBACK));
 }
 
+/**
+ * __snd_pcm_set_state - Change the current PCM state
+ * @runtime: PCM runtime to set
+ * @state: the current state to set
+ *
+ * Call within the stream lock
+ */
+static inline void __snd_pcm_set_state(struct snd_pcm_runtime *runtime,
+				       snd_pcm_state_t state)
+{
+	runtime->state = state;
+	runtime->status->state = state; /* copy for mmap */
+}
+
 /**
  * bytes_to_samples - Unit conversion of the size from bytes to samples
  * @runtime: PCM runtime instance
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index 02df915eb3c6..ac2efeb63a39 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -1237,12 +1237,12 @@ snd_pcm_sframes_t snd_pcm_oss_write3(struct snd_pcm_substream *substream, const
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	int ret;
 	while (1) {
-		if (runtime->status->state == SNDRV_PCM_STATE_XRUN ||
-		    runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) {
+		if (runtime->state == SNDRV_PCM_STATE_XRUN ||
+		    runtime->state == SNDRV_PCM_STATE_SUSPENDED) {
 #ifdef OSS_DEBUG
 			pcm_dbg(substream->pcm,
 				"pcm_oss: write: recovering from %s\n",
-				runtime->status->state == SNDRV_PCM_STATE_XRUN ?
+				runtime->state == SNDRV_PCM_STATE_XRUN ?
 				"XRUN" : "SUSPEND");
 #endif
 			ret = snd_pcm_oss_prepare(substream);
@@ -1257,7 +1257,7 @@ snd_pcm_sframes_t snd_pcm_oss_write3(struct snd_pcm_substream *substream, const
 			break;
 		/* test, if we can't store new data, because the stream */
 		/* has not been started */
-		if (runtime->status->state == SNDRV_PCM_STATE_PREPARED)
+		if (runtime->state == SNDRV_PCM_STATE_PREPARED)
 			return -EAGAIN;
 	}
 	return ret;
@@ -1269,18 +1269,18 @@ snd_pcm_sframes_t snd_pcm_oss_read3(struct snd_pcm_substream *substream, char *p
 	snd_pcm_sframes_t delay;
 	int ret;
 	while (1) {
-		if (runtime->status->state == SNDRV_PCM_STATE_XRUN ||
-		    runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) {
+		if (runtime->state == SNDRV_PCM_STATE_XRUN ||
+		    runtime->state == SNDRV_PCM_STATE_SUSPENDED) {
 #ifdef OSS_DEBUG
 			pcm_dbg(substream->pcm,
 				"pcm_oss: read: recovering from %s\n",
-				runtime->status->state == SNDRV_PCM_STATE_XRUN ?
+				runtime->state == SNDRV_PCM_STATE_XRUN ?
 				"XRUN" : "SUSPEND");
 #endif
 			ret = snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_DRAIN, NULL);
 			if (ret < 0)
 				break;
-		} else if (runtime->status->state == SNDRV_PCM_STATE_SETUP) {
+		} else if (runtime->state == SNDRV_PCM_STATE_SETUP) {
 			ret = snd_pcm_oss_prepare(substream);
 			if (ret < 0)
 				break;
@@ -1293,7 +1293,7 @@ snd_pcm_sframes_t snd_pcm_oss_read3(struct snd_pcm_substream *substream, char *p
 					 frames, in_kernel);
 		mutex_lock(&runtime->oss.params_lock);
 		if (ret == -EPIPE) {
-			if (runtime->status->state == SNDRV_PCM_STATE_DRAINING) {
+			if (runtime->state == SNDRV_PCM_STATE_DRAINING) {
 				ret = snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_DROP, NULL);
 				if (ret < 0)
 					break;
@@ -1312,12 +1312,12 @@ snd_pcm_sframes_t snd_pcm_oss_writev3(struct snd_pcm_substream *substream, void
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	int ret;
 	while (1) {
-		if (runtime->status->state == SNDRV_PCM_STATE_XRUN ||
-		    runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) {
+		if (runtime->state == SNDRV_PCM_STATE_XRUN ||
+		    runtime->state == SNDRV_PCM_STATE_SUSPENDED) {
 #ifdef OSS_DEBUG
 			pcm_dbg(substream->pcm,
 				"pcm_oss: writev: recovering from %s\n",
-				runtime->status->state == SNDRV_PCM_STATE_XRUN ?
+				runtime->state == SNDRV_PCM_STATE_XRUN ?
 				"XRUN" : "SUSPEND");
 #endif
 			ret = snd_pcm_oss_prepare(substream);
@@ -1330,7 +1330,7 @@ snd_pcm_sframes_t snd_pcm_oss_writev3(struct snd_pcm_substream *substream, void
 
 		/* test, if we can't store new data, because the stream */
 		/* has not been started */
-		if (runtime->status->state == SNDRV_PCM_STATE_PREPARED)
+		if (runtime->state == SNDRV_PCM_STATE_PREPARED)
 			return -EAGAIN;
 	}
 	return ret;
@@ -1341,18 +1341,18 @@ snd_pcm_sframes_t snd_pcm_oss_readv3(struct snd_pcm_substream *substream, void *
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	int ret;
 	while (1) {
-		if (runtime->status->state == SNDRV_PCM_STATE_XRUN ||
-		    runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) {
+		if (runtime->state == SNDRV_PCM_STATE_XRUN ||
+		    runtime->state == SNDRV_PCM_STATE_SUSPENDED) {
 #ifdef OSS_DEBUG
 			pcm_dbg(substream->pcm,
 				"pcm_oss: readv: recovering from %s\n",
-				runtime->status->state == SNDRV_PCM_STATE_XRUN ?
+				runtime->state == SNDRV_PCM_STATE_XRUN ?
 				"XRUN" : "SUSPEND");
 #endif
 			ret = snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_DRAIN, NULL);
 			if (ret < 0)
 				break;
-		} else if (runtime->status->state == SNDRV_PCM_STATE_SETUP) {
+		} else if (runtime->state == SNDRV_PCM_STATE_SETUP) {
 			ret = snd_pcm_oss_prepare(substream);
 			if (ret < 0)
 				break;
@@ -1635,7 +1635,7 @@ static int snd_pcm_oss_sync1(struct snd_pcm_substream *substream, size_t size)
 		result = 0;
 		set_current_state(TASK_INTERRUPTIBLE);
 		snd_pcm_stream_lock_irq(substream);
-		state = runtime->status->state;
+		state = runtime->state;
 		snd_pcm_stream_unlock_irq(substream);
 		if (state != SNDRV_PCM_STATE_RUNNING) {
 			set_current_state(TASK_RUNNING);
@@ -2854,8 +2854,8 @@ static __poll_t snd_pcm_oss_poll(struct file *file, poll_table * wait)
 		struct snd_pcm_runtime *runtime = psubstream->runtime;
 		poll_wait(file, &runtime->sleep, wait);
 		snd_pcm_stream_lock_irq(psubstream);
-		if (runtime->status->state != SNDRV_PCM_STATE_DRAINING &&
-		    (runtime->status->state != SNDRV_PCM_STATE_RUNNING ||
+		if (runtime->state != SNDRV_PCM_STATE_DRAINING &&
+		    (runtime->state != SNDRV_PCM_STATE_RUNNING ||
 		     snd_pcm_oss_playback_ready(psubstream)))
 			mask |= EPOLLOUT | EPOLLWRNORM;
 		snd_pcm_stream_unlock_irq(psubstream);
@@ -2865,7 +2865,7 @@ static __poll_t snd_pcm_oss_poll(struct file *file, poll_table * wait)
 		snd_pcm_state_t ostate;
 		poll_wait(file, &runtime->sleep, wait);
 		snd_pcm_stream_lock_irq(csubstream);
-		ostate = runtime->status->state;
+		ostate = runtime->state;
 		if (ostate != SNDRV_PCM_STATE_RUNNING ||
 		    snd_pcm_oss_capture_ready(csubstream))
 			mask |= EPOLLIN | EPOLLRDNORM;
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 82925709fa12..9d95e3731123 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -387,7 +387,7 @@ static void snd_pcm_substream_proc_hw_params_read(struct snd_info_entry *entry,
 		snd_iprintf(buffer, "closed\n");
 		goto unlock;
 	}
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+	if (runtime->state == SNDRV_PCM_STATE_OPEN) {
 		snd_iprintf(buffer, "no setup\n");
 		goto unlock;
 	}
@@ -424,7 +424,7 @@ static void snd_pcm_substream_proc_sw_params_read(struct snd_info_entry *entry,
 		snd_iprintf(buffer, "closed\n");
 		goto unlock;
 	}
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+	if (runtime->state == SNDRV_PCM_STATE_OPEN) {
 		snd_iprintf(buffer, "no setup\n");
 		goto unlock;
 	}
@@ -970,7 +970,7 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
 	init_waitqueue_head(&runtime->sleep);
 	init_waitqueue_head(&runtime->tsleep);
 
-	runtime->status->state = SNDRV_PCM_STATE_OPEN;
+	__snd_pcm_set_state(runtime, SNDRV_PCM_STATE_OPEN);
 	mutex_init(&runtime->buffer_mutex);
 	atomic_set(&runtime->buffer_accessing, 0);
 
@@ -1112,7 +1112,8 @@ static int snd_pcm_dev_disconnect(struct snd_device *device)
 			if (snd_pcm_running(substream))
 				snd_pcm_stop(substream, SNDRV_PCM_STATE_DISCONNECTED);
 			/* to be sure, set the state unconditionally */
-			substream->runtime->status->state = SNDRV_PCM_STATE_DISCONNECTED;
+			__snd_pcm_set_state(substream->runtime,
+					    SNDRV_PCM_STATE_DISCONNECTED);
 			wake_up(&substream->runtime->sleep);
 			wake_up(&substream->runtime->tsleep);
 		}
diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c
index 917c5b4f19d7..42c2ada8e888 100644
--- a/sound/core/pcm_compat.c
+++ b/sound/core/pcm_compat.c
@@ -295,7 +295,7 @@ static int snd_pcm_ioctl_xferi_compat(struct snd_pcm_substream *substream,
 		return -ENOTTY;
 	if (substream->stream != dir)
 		return -EINVAL;
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN)
+	if (substream->runtime->state == SNDRV_PCM_STATE_OPEN)
 		return -EBADFD;
 
 	if (get_user(buf, &data32->buf) ||
@@ -341,7 +341,7 @@ static int snd_pcm_ioctl_xfern_compat(struct snd_pcm_substream *substream,
 		return -ENOTTY;
 	if (substream->stream != dir)
 		return -EINVAL;
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN)
+	if (substream->runtime->state == SNDRV_PCM_STATE_OPEN)
 		return -EBADFD;
 
 	ch = substream->runtime->channels;
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 40751e5aff09..8b6aeb8a78f7 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -186,7 +186,7 @@ int snd_pcm_update_state(struct snd_pcm_substream *substream,
 	avail = snd_pcm_avail(substream);
 	if (avail > runtime->avail_max)
 		runtime->avail_max = avail;
-	if (runtime->status->state == SNDRV_PCM_STATE_DRAINING) {
+	if (runtime->state == SNDRV_PCM_STATE_DRAINING) {
 		if (avail >= runtime->buffer_size) {
 			snd_pcm_drain_done(substream);
 			return -EPIPE;
@@ -1911,7 +1911,7 @@ static int wait_for_avail(struct snd_pcm_substream *substream,
 
 		snd_pcm_stream_lock_irq(substream);
 		set_current_state(TASK_INTERRUPTIBLE);
-		switch (runtime->status->state) {
+		switch (runtime->state) {
 		case SNDRV_PCM_STATE_SUSPENDED:
 			err = -ESTRPIPE;
 			goto _endloop;
@@ -2099,14 +2099,14 @@ static int pcm_sanity_check(struct snd_pcm_substream *substream)
 	runtime = substream->runtime;
 	if (snd_BUG_ON(!substream->ops->copy_user && !runtime->dma_area))
 		return -EINVAL;
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN)
+	if (runtime->state == SNDRV_PCM_STATE_OPEN)
 		return -EBADFD;
 	return 0;
 }
 
 static int pcm_accessible_state(struct snd_pcm_runtime *runtime)
 {
-	switch (runtime->status->state) {
+	switch (runtime->state) {
 	case SNDRV_PCM_STATE_PREPARED:
 	case SNDRV_PCM_STATE_RUNNING:
 	case SNDRV_PCM_STATE_PAUSED:
@@ -2225,7 +2225,7 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
 		goto _end_unlock;
 
 	runtime->twake = runtime->control->avail_min ? : 1;
-	if (runtime->status->state == SNDRV_PCM_STATE_RUNNING)
+	if (runtime->state == SNDRV_PCM_STATE_RUNNING)
 		snd_pcm_update_hw_ptr(substream);
 
 	/*
@@ -2233,7 +2233,7 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
 	 * thread may start capture
 	 */
 	if (!is_playback &&
-	    runtime->status->state == SNDRV_PCM_STATE_PREPARED &&
+	    runtime->state == SNDRV_PCM_STATE_PREPARED &&
 	    size >= runtime->start_threshold) {
 		err = snd_pcm_start(substream);
 		if (err < 0)
@@ -2247,7 +2247,7 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
 		snd_pcm_uframes_t cont;
 		if (!avail) {
 			if (!is_playback &&
-			    runtime->status->state == SNDRV_PCM_STATE_DRAINING) {
+			    runtime->state == SNDRV_PCM_STATE_DRAINING) {
 				snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP);
 				goto _end_unlock;
 			}
@@ -2303,7 +2303,7 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
 		xfer += frames;
 		avail -= frames;
 		if (is_playback &&
-		    runtime->status->state == SNDRV_PCM_STATE_PREPARED &&
+		    runtime->state == SNDRV_PCM_STATE_PREPARED &&
 		    snd_pcm_playback_hw_avail(runtime) >= (snd_pcm_sframes_t)runtime->start_threshold) {
 			err = snd_pcm_start(substream);
 			if (err < 0)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index ad0541e9e888..d9485b1ab719 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -595,8 +595,8 @@ static void snd_pcm_set_state(struct snd_pcm_substream *substream,
 			      snd_pcm_state_t state)
 {
 	snd_pcm_stream_lock_irq(substream);
-	if (substream->runtime->status->state != SNDRV_PCM_STATE_DISCONNECTED)
-		substream->runtime->status->state = state;
+	if (substream->runtime->state != SNDRV_PCM_STATE_DISCONNECTED)
+		__snd_pcm_set_state(substream->runtime, state);
 	snd_pcm_stream_unlock_irq(substream);
 }
 
@@ -724,7 +724,7 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
 	if (err < 0)
 		return err;
 	snd_pcm_stream_lock_irq(substream);
-	switch (runtime->status->state) {
+	switch (runtime->state) {
 	case SNDRV_PCM_STATE_OPEN:
 	case SNDRV_PCM_STATE_SETUP:
 	case SNDRV_PCM_STATE_PREPARED:
@@ -889,7 +889,7 @@ static int snd_pcm_hw_free(struct snd_pcm_substream *substream)
 	if (result < 0)
 		return result;
 	snd_pcm_stream_lock_irq(substream);
-	switch (runtime->status->state) {
+	switch (runtime->state) {
 	case SNDRV_PCM_STATE_SETUP:
 	case SNDRV_PCM_STATE_PREPARED:
 		if (atomic_read(&substream->mmap_count))
@@ -920,7 +920,7 @@ static int snd_pcm_sw_params(struct snd_pcm_substream *substream,
 		return -ENXIO;
 	runtime = substream->runtime;
 	snd_pcm_stream_lock_irq(substream);
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+	if (runtime->state == SNDRV_PCM_STATE_OPEN) {
 		snd_pcm_stream_unlock_irq(substream);
 		return -EBADFD;
 	}
@@ -1013,8 +1013,8 @@ int snd_pcm_status64(struct snd_pcm_substream *substream,
 	} else
 		runtime->audio_tstamp_report.valid = 1;
 
-	status->state = runtime->status->state;
-	status->suspended_state = runtime->status->suspended_state;
+	status->state = runtime->state;
+	status->suspended_state = runtime->suspended_state;
 	if (status->state == SNDRV_PCM_STATE_OPEN)
 		goto _end;
 	status->trigger_tstamp_sec = runtime->trigger_tstamp.tv_sec;
@@ -1148,7 +1148,7 @@ static int snd_pcm_channel_info(struct snd_pcm_substream *substream,
 	channel = info->channel;
 	runtime = substream->runtime;
 	snd_pcm_stream_lock_irq(substream);
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+	if (runtime->state == SNDRV_PCM_STATE_OPEN) {
 		snd_pcm_stream_unlock_irq(substream);
 		return -EBADFD;
 	}
@@ -1411,7 +1411,7 @@ static int snd_pcm_pre_start(struct snd_pcm_substream *substream,
 			     snd_pcm_state_t state)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	if (runtime->status->state != SNDRV_PCM_STATE_PREPARED)
+	if (runtime->state != SNDRV_PCM_STATE_PREPARED)
 		return -EBADFD;
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
 	    !snd_pcm_playback_data(substream))
@@ -1444,7 +1444,7 @@ static void snd_pcm_post_start(struct snd_pcm_substream *substream,
 	runtime->hw_ptr_jiffies = jiffies;
 	runtime->hw_ptr_buffer_jiffies = (runtime->buffer_size * HZ) / 
 							    runtime->rate;
-	runtime->status->state = state;
+	__snd_pcm_set_state(runtime, state);
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
 	    runtime->silence_size > 0)
 		snd_pcm_playback_silence(substream, ULONG_MAX);
@@ -1485,7 +1485,7 @@ static int snd_pcm_pre_stop(struct snd_pcm_substream *substream,
 			    snd_pcm_state_t state)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN)
+	if (runtime->state == SNDRV_PCM_STATE_OPEN)
 		return -EBADFD;
 	runtime->trigger_master = substream;
 	return 0;
@@ -1506,9 +1506,9 @@ static void snd_pcm_post_stop(struct snd_pcm_substream *substream,
 			      snd_pcm_state_t state)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	if (runtime->status->state != state) {
+	if (runtime->state != state) {
 		snd_pcm_trigger_tstamp(substream);
-		runtime->status->state = state;
+		__snd_pcm_set_state(runtime, state);
 		snd_pcm_timer_notify(substream, SNDRV_TIMER_EVENT_MSTOP);
 	}
 	wake_up(&runtime->sleep);
@@ -1584,9 +1584,9 @@ static int snd_pcm_pre_pause(struct snd_pcm_substream *substream,
 	if (!(runtime->info & SNDRV_PCM_INFO_PAUSE))
 		return -ENOSYS;
 	if (pause_pushed(state)) {
-		if (runtime->status->state != SNDRV_PCM_STATE_RUNNING)
+		if (runtime->state != SNDRV_PCM_STATE_RUNNING)
 			return -EBADFD;
-	} else if (runtime->status->state != SNDRV_PCM_STATE_PAUSED)
+	} else if (runtime->state != SNDRV_PCM_STATE_PAUSED)
 		return -EBADFD;
 	runtime->trigger_master = substream;
 	return 0;
@@ -1628,12 +1628,12 @@ static void snd_pcm_post_pause(struct snd_pcm_substream *substream,
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	snd_pcm_trigger_tstamp(substream);
 	if (pause_pushed(state)) {
-		runtime->status->state = SNDRV_PCM_STATE_PAUSED;
+		__snd_pcm_set_state(runtime, SNDRV_PCM_STATE_PAUSED);
 		snd_pcm_timer_notify(substream, SNDRV_TIMER_EVENT_MPAUSE);
 		wake_up(&runtime->sleep);
 		wake_up(&runtime->tsleep);
 	} else {
-		runtime->status->state = SNDRV_PCM_STATE_RUNNING;
+		__snd_pcm_set_state(runtime, SNDRV_PCM_STATE_RUNNING);
 		snd_pcm_timer_notify(substream, SNDRV_TIMER_EVENT_MCONTINUE);
 	}
 }
@@ -1668,7 +1668,7 @@ static int snd_pcm_pre_suspend(struct snd_pcm_substream *substream,
 			       snd_pcm_state_t state)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	switch (runtime->status->state) {
+	switch (runtime->state) {
 	case SNDRV_PCM_STATE_SUSPENDED:
 		return -EBUSY;
 	/* unresumable PCM state; return -EBUSY for skipping suspend */
@@ -1699,8 +1699,9 @@ static void snd_pcm_post_suspend(struct snd_pcm_substream *substream,
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	snd_pcm_trigger_tstamp(substream);
-	runtime->status->suspended_state = runtime->status->state;
-	runtime->status->state = SNDRV_PCM_STATE_SUSPENDED;
+	runtime->suspended_state = runtime->state;
+	runtime->status->suspended_state = runtime->suspended_state;
+	__snd_pcm_set_state(runtime, SNDRV_PCM_STATE_SUSPENDED);
 	snd_pcm_timer_notify(substream, SNDRV_TIMER_EVENT_MSUSPEND);
 	wake_up(&runtime->sleep);
 	wake_up(&runtime->tsleep);
@@ -1791,8 +1792,8 @@ static int snd_pcm_do_resume(struct snd_pcm_substream *substream,
 	if (runtime->trigger_master != substream)
 		return 0;
 	/* DMA not running previously? */
-	if (runtime->status->suspended_state != SNDRV_PCM_STATE_RUNNING &&
-	    (runtime->status->suspended_state != SNDRV_PCM_STATE_DRAINING ||
+	if (runtime->suspended_state != SNDRV_PCM_STATE_RUNNING &&
+	    (runtime->suspended_state != SNDRV_PCM_STATE_DRAINING ||
 	     substream->stream != SNDRV_PCM_STREAM_PLAYBACK))
 		return 0;
 	return substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_RESUME);
@@ -1811,7 +1812,7 @@ static void snd_pcm_post_resume(struct snd_pcm_substream *substream,
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	snd_pcm_trigger_tstamp(substream);
-	runtime->status->state = runtime->status->suspended_state;
+	__snd_pcm_set_state(runtime, runtime->suspended_state);
 	snd_pcm_timer_notify(substream, SNDRV_TIMER_EVENT_MRESUME);
 }
 
@@ -1848,7 +1849,7 @@ static int snd_pcm_xrun(struct snd_pcm_substream *substream)
 	int result;
 
 	snd_pcm_stream_lock_irq(substream);
-	switch (runtime->status->state) {
+	switch (runtime->state) {
 	case SNDRV_PCM_STATE_XRUN:
 		result = 0;	/* already there */
 		break;
@@ -1871,7 +1872,7 @@ static int snd_pcm_pre_reset(struct snd_pcm_substream *substream,
 			     snd_pcm_state_t state)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	switch (runtime->status->state) {
+	switch (runtime->state) {
 	case SNDRV_PCM_STATE_RUNNING:
 	case SNDRV_PCM_STATE_PREPARED:
 	case SNDRV_PCM_STATE_PAUSED:
@@ -1933,8 +1934,8 @@ static int snd_pcm_pre_prepare(struct snd_pcm_substream *substream,
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	int f_flags = (__force int)state;
 
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN ||
-	    runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED)
+	if (runtime->state == SNDRV_PCM_STATE_OPEN ||
+	    runtime->state == SNDRV_PCM_STATE_DISCONNECTED)
 		return -EBADFD;
 	if (snd_pcm_running(substream))
 		return -EBUSY;
@@ -1985,7 +1986,7 @@ static int snd_pcm_prepare(struct snd_pcm_substream *substream,
 		f_flags = substream->f_flags;
 
 	snd_pcm_stream_lock_irq(substream);
-	switch (substream->runtime->status->state) {
+	switch (substream->runtime->state) {
 	case SNDRV_PCM_STATE_PAUSED:
 		snd_pcm_pause(substream, false);
 		fallthrough;
@@ -2009,7 +2010,7 @@ static int snd_pcm_pre_drain_init(struct snd_pcm_substream *substream,
 				  snd_pcm_state_t state)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	switch (runtime->status->state) {
+	switch (runtime->state) {
 	case SNDRV_PCM_STATE_OPEN:
 	case SNDRV_PCM_STATE_DISCONNECTED:
 	case SNDRV_PCM_STATE_SUSPENDED:
@@ -2024,28 +2025,28 @@ static int snd_pcm_do_drain_init(struct snd_pcm_substream *substream,
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		switch (runtime->status->state) {
+		switch (runtime->state) {
 		case SNDRV_PCM_STATE_PREPARED:
 			/* start playback stream if possible */
 			if (! snd_pcm_playback_empty(substream)) {
 				snd_pcm_do_start(substream, SNDRV_PCM_STATE_DRAINING);
 				snd_pcm_post_start(substream, SNDRV_PCM_STATE_DRAINING);
 			} else {
-				runtime->status->state = SNDRV_PCM_STATE_SETUP;
+				__snd_pcm_set_state(runtime, SNDRV_PCM_STATE_SETUP);
 			}
 			break;
 		case SNDRV_PCM_STATE_RUNNING:
-			runtime->status->state = SNDRV_PCM_STATE_DRAINING;
+			__snd_pcm_set_state(runtime, SNDRV_PCM_STATE_DRAINING);
 			break;
 		case SNDRV_PCM_STATE_XRUN:
-			runtime->status->state = SNDRV_PCM_STATE_SETUP;
+			__snd_pcm_set_state(runtime, SNDRV_PCM_STATE_SETUP);
 			break;
 		default:
 			break;
 		}
 	} else {
 		/* stop running stream */
-		if (runtime->status->state == SNDRV_PCM_STATE_RUNNING) {
+		if (runtime->state == SNDRV_PCM_STATE_RUNNING) {
 			snd_pcm_state_t new_state;
 
 			new_state = snd_pcm_capture_avail(runtime) > 0 ?
@@ -2055,7 +2056,7 @@ static int snd_pcm_do_drain_init(struct snd_pcm_substream *substream,
 		}
 	}
 
-	if (runtime->status->state == SNDRV_PCM_STATE_DRAINING &&
+	if (runtime->state == SNDRV_PCM_STATE_DRAINING &&
 	    runtime->trigger_master == substream &&
 	    (runtime->hw.info & SNDRV_PCM_INFO_DRAIN_TRIGGER))
 		return substream->ops->trigger(substream,
@@ -2096,7 +2097,7 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
 	card = substream->pcm->card;
 	runtime = substream->runtime;
 
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN)
+	if (runtime->state == SNDRV_PCM_STATE_OPEN)
 		return -EBADFD;
 
 	if (file) {
@@ -2107,7 +2108,7 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
 
 	snd_pcm_stream_lock_irq(substream);
 	/* resume pause */
-	if (runtime->status->state == SNDRV_PCM_STATE_PAUSED)
+	if (runtime->state == SNDRV_PCM_STATE_PAUSED)
 		snd_pcm_pause(substream, false);
 
 	/* pre-start/stop - all running streams are changed to DRAINING state */
@@ -2135,7 +2136,7 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
 			if (s->stream != SNDRV_PCM_STREAM_PLAYBACK)
 				continue;
 			runtime = s->runtime;
-			if (runtime->status->state == SNDRV_PCM_STATE_DRAINING) {
+			if (runtime->state == SNDRV_PCM_STATE_DRAINING) {
 				to_check = runtime;
 				break;
 			}
@@ -2174,7 +2175,7 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
 			break;
 		}
 		if (tout == 0) {
-			if (substream->runtime->status->state == SNDRV_PCM_STATE_SUSPENDED)
+			if (substream->runtime->state == SNDRV_PCM_STATE_SUSPENDED)
 				result = -ESTRPIPE;
 			else {
 				dev_dbg(substream->pcm->card->dev,
@@ -2206,13 +2207,13 @@ static int snd_pcm_drop(struct snd_pcm_substream *substream)
 		return -ENXIO;
 	runtime = substream->runtime;
 
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN ||
-	    runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED)
+	if (runtime->state == SNDRV_PCM_STATE_OPEN ||
+	    runtime->state == SNDRV_PCM_STATE_DISCONNECTED)
 		return -EBADFD;
 
 	snd_pcm_stream_lock_irq(substream);
 	/* resume pause */
-	if (runtime->status->state == SNDRV_PCM_STATE_PAUSED)
+	if (runtime->state == SNDRV_PCM_STATE_PAUSED)
 		snd_pcm_pause(substream, false);
 
 	snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP);
@@ -2275,8 +2276,8 @@ static int snd_pcm_link(struct snd_pcm_substream *substream, int fd)
 	snd_pcm_group_init(group);
 
 	down_write(&snd_pcm_link_rwsem);
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN ||
-	    substream->runtime->status->state != substream1->runtime->status->state ||
+	if (substream->runtime->state == SNDRV_PCM_STATE_OPEN ||
+	    substream->runtime->state != substream1->runtime->state ||
 	    substream->pcm->nonatomic != substream1->pcm->nonatomic) {
 		res = -EBADFD;
 		goto _end;
@@ -2700,7 +2701,7 @@ void snd_pcm_release_substream(struct snd_pcm_substream *substream)
 
 	snd_pcm_drop(substream);
 	if (substream->hw_opened) {
-		if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+		if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
 			do_hw_free(substream);
 		substream->ops->close(substream);
 		substream->hw_opened = 0;
@@ -2904,7 +2905,7 @@ static int snd_pcm_release(struct inode *inode, struct file *file)
  */
 static int do_pcm_hwsync(struct snd_pcm_substream *substream)
 {
-	switch (substream->runtime->status->state) {
+	switch (substream->runtime->state) {
 	case SNDRV_PCM_STATE_DRAINING:
 		if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
 			return -EBADFD;
@@ -3203,7 +3204,7 @@ static int snd_pcm_xferi_frames_ioctl(struct snd_pcm_substream *substream,
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	snd_pcm_sframes_t result;
 
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN)
+	if (runtime->state == SNDRV_PCM_STATE_OPEN)
 		return -EBADFD;
 	if (put_user(0, &_xferi->result))
 		return -EFAULT;
@@ -3226,7 +3227,7 @@ static int snd_pcm_xfern_frames_ioctl(struct snd_pcm_substream *substream,
 	void *bufs;
 	snd_pcm_sframes_t result;
 
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN)
+	if (runtime->state == SNDRV_PCM_STATE_OPEN)
 		return -EBADFD;
 	if (runtime->channels > 128)
 		return -EINVAL;
@@ -3290,7 +3291,7 @@ static int snd_pcm_common_ioctl(struct file *file,
 	if (PCM_RUNTIME_CHECK(substream))
 		return -ENXIO;
 
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED)
+	if (substream->runtime->state == SNDRV_PCM_STATE_DISCONNECTED)
 		return -EBADFD;
 
 	res = snd_power_wait(substream->pcm->card);
@@ -3421,7 +3422,7 @@ int snd_pcm_kernel_ioctl(struct snd_pcm_substream *substream,
 	snd_pcm_uframes_t *frames = arg;
 	snd_pcm_sframes_t result;
 	
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED)
+	if (substream->runtime->state == SNDRV_PCM_STATE_DISCONNECTED)
 		return -EBADFD;
 
 	switch (cmd) {
@@ -3466,8 +3467,8 @@ static ssize_t snd_pcm_read(struct file *file, char __user *buf, size_t count,
 	if (PCM_RUNTIME_CHECK(substream))
 		return -ENXIO;
 	runtime = substream->runtime;
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN ||
-	    runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED)
+	if (runtime->state == SNDRV_PCM_STATE_OPEN ||
+	    runtime->state == SNDRV_PCM_STATE_DISCONNECTED)
 		return -EBADFD;
 	if (!frame_aligned(runtime, count))
 		return -EINVAL;
@@ -3491,8 +3492,8 @@ static ssize_t snd_pcm_write(struct file *file, const char __user *buf,
 	if (PCM_RUNTIME_CHECK(substream))
 		return -ENXIO;
 	runtime = substream->runtime;
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN ||
-	    runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED)
+	if (runtime->state == SNDRV_PCM_STATE_OPEN ||
+	    runtime->state == SNDRV_PCM_STATE_DISCONNECTED)
 		return -EBADFD;
 	if (!frame_aligned(runtime, count))
 		return -EINVAL;
@@ -3518,8 +3519,8 @@ static ssize_t snd_pcm_readv(struct kiocb *iocb, struct iov_iter *to)
 	if (PCM_RUNTIME_CHECK(substream))
 		return -ENXIO;
 	runtime = substream->runtime;
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN ||
-	    runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED)
+	if (runtime->state == SNDRV_PCM_STATE_OPEN ||
+	    runtime->state == SNDRV_PCM_STATE_DISCONNECTED)
 		return -EBADFD;
 	if (!iter_is_iovec(to))
 		return -EINVAL;
@@ -3555,8 +3556,8 @@ static ssize_t snd_pcm_writev(struct kiocb *iocb, struct iov_iter *from)
 	if (PCM_RUNTIME_CHECK(substream))
 		return -ENXIO;
 	runtime = substream->runtime;
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN ||
-	    runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED)
+	if (runtime->state == SNDRV_PCM_STATE_OPEN ||
+	    runtime->state == SNDRV_PCM_STATE_DISCONNECTED)
 		return -EBADFD;
 	if (!iter_is_iovec(from))
 		return -EINVAL;
@@ -3595,7 +3596,7 @@ static __poll_t snd_pcm_poll(struct file *file, poll_table *wait)
 		return ok | EPOLLERR;
 
 	runtime = substream->runtime;
-	if (runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED)
+	if (runtime->state == SNDRV_PCM_STATE_DISCONNECTED)
 		return ok | EPOLLERR;
 
 	poll_wait(file, &runtime->sleep, wait);
@@ -3603,7 +3604,7 @@ static __poll_t snd_pcm_poll(struct file *file, poll_table *wait)
 	mask = 0;
 	snd_pcm_stream_lock_irq(substream);
 	avail = snd_pcm_avail(substream);
-	switch (runtime->status->state) {
+	switch (runtime->state) {
 	case SNDRV_PCM_STATE_RUNNING:
 	case SNDRV_PCM_STATE_PREPARED:
 	case SNDRV_PCM_STATE_PAUSED:
@@ -3874,7 +3875,7 @@ int snd_pcm_mmap_data(struct snd_pcm_substream *substream, struct file *file,
 			return -EINVAL;
 	}
 	runtime = substream->runtime;
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN)
+	if (runtime->state == SNDRV_PCM_STATE_OPEN)
 		return -EBADFD;
 	if (!(runtime->info & SNDRV_PCM_INFO_MMAP))
 		return -ENXIO;
@@ -3911,7 +3912,7 @@ static int snd_pcm_mmap(struct file *file, struct vm_area_struct *area)
 	substream = pcm_file->substream;
 	if (PCM_RUNTIME_CHECK(substream))
 		return -ENXIO;
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED)
+	if (substream->runtime->state == SNDRV_PCM_STATE_DISCONNECTED)
 		return -EBADFD;
 
 	offset = area->vm_pgoff << PAGE_SHIFT;
@@ -3949,7 +3950,7 @@ static int snd_pcm_fasync(int fd, struct file * file, int on)
 	if (PCM_RUNTIME_CHECK(substream))
 		return -ENXIO;
 	runtime = substream->runtime;
-	if (runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED)
+	if (runtime->state == SNDRV_PCM_STATE_DISCONNECTED)
 		return -EBADFD;
 	return snd_fasync_helper(fd, file, on, &runtime->fasync);
 }
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 02/11] ALSA: pcm: Make mmap status read-only
  2022-09-26 13:55 [PATCH 00/11] ALSA: PCM state reference optimization Takashi Iwai
  2022-09-26 13:55 ` [PATCH 01/11] ALSA: pcm: Avoid reference to status->state Takashi Iwai
@ 2022-09-26 13:55 ` Takashi Iwai
  2022-09-26 13:55 ` [PATCH 03/11] ALSA: aloop: Replace runtime->status->state reference to runtime->state Takashi Iwai
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2022-09-26 13:55 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai

The mmap status record should be read-only.  Modifying it from
user-space may screw up things unexpectedly, so let's clear the write
bits at exposing it.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/pcm_native.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index d9485b1ab719..33769ca78cc8 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -3668,6 +3668,7 @@ static int snd_pcm_mmap_status(struct snd_pcm_substream *substream, struct file
 	area->vm_ops = &snd_pcm_vm_ops_status;
 	area->vm_private_data = substream;
 	area->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
+	area->vm_flags &= ~(VM_WRITE | VM_MAYWRITE);
 	return 0;
 }
 
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 03/11] ALSA: aloop: Replace runtime->status->state reference to runtime->state
  2022-09-26 13:55 [PATCH 00/11] ALSA: PCM state reference optimization Takashi Iwai
  2022-09-26 13:55 ` [PATCH 01/11] ALSA: pcm: Avoid reference to status->state Takashi Iwai
  2022-09-26 13:55 ` [PATCH 02/11] ALSA: pcm: Make mmap status read-only Takashi Iwai
@ 2022-09-26 13:55 ` Takashi Iwai
  2022-09-26 13:55 ` [PATCH 04/11] ALSA: firewire: " Takashi Iwai
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2022-09-26 13:55 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai

The recent change in ALSA core allows drivers to get the current PCM
state directly from runtime object.  Replace the calls accordingly.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/drivers/aloop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/drivers/aloop.c b/sound/drivers/aloop.c
index 12f12a294df5..a38e602b4fc6 100644
--- a/sound/drivers/aloop.c
+++ b/sound/drivers/aloop.c
@@ -535,7 +535,7 @@ static void copy_play_buf(struct loopback_pcm *play,
 
 	/* check if playback is draining, trim the capture copy size
 	 * when our pointer is at the end of playback ring buffer */
-	if (runtime->status->state == SNDRV_PCM_STATE_DRAINING &&
+	if (runtime->state == SNDRV_PCM_STATE_DRAINING &&
 	    snd_pcm_playback_hw_avail(runtime) < runtime->buffer_size) { 
 	    	snd_pcm_uframes_t appl_ptr, appl_ptr1, diff;
 		appl_ptr = appl_ptr1 = runtime->control->appl_ptr;
@@ -730,7 +730,7 @@ static void loopback_snd_timer_period_elapsed(struct loopback_cable *cable,
 
 	if (event == SNDRV_TIMER_EVENT_MSTOP) {
 		if (!dpcm_play ||
-		    dpcm_play->substream->runtime->status->state !=
+		    dpcm_play->substream->runtime->state !=
 				SNDRV_PCM_STATE_DRAINING) {
 			spin_unlock_irqrestore(&cable->lock, flags);
 			return;
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 04/11] ALSA: firewire: Replace runtime->status->state reference to runtime->state
  2022-09-26 13:55 [PATCH 00/11] ALSA: PCM state reference optimization Takashi Iwai
                   ` (2 preceding siblings ...)
  2022-09-26 13:55 ` [PATCH 03/11] ALSA: aloop: Replace runtime->status->state reference to runtime->state Takashi Iwai
@ 2022-09-26 13:55 ` Takashi Iwai
  2022-09-26 13:55 ` [PATCH 05/11] ALSA: hda: " Takashi Iwai
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2022-09-26 13:55 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai

The recent change in ALSA core allows drivers to get the current PCM
state directly from runtime object.  Replace the calls accordingly.

Cc: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/firewire/bebob/bebob_pcm.c         | 4 ++--
 sound/firewire/dice/dice-pcm.c           | 4 ++--
 sound/firewire/digi00x/digi00x-pcm.c     | 4 ++--
 sound/firewire/fireface/ff-pcm.c         | 4 ++--
 sound/firewire/fireworks/fireworks_pcm.c | 4 ++--
 sound/firewire/motu/motu-pcm.c           | 4 ++--
 sound/firewire/oxfw/oxfw-pcm.c           | 8 ++++----
 sound/firewire/tascam/tascam-pcm.c       | 4 ++--
 8 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/sound/firewire/bebob/bebob_pcm.c b/sound/firewire/bebob/bebob_pcm.c
index f8d9a2041264..ce49eef0fcba 100644
--- a/sound/firewire/bebob/bebob_pcm.c
+++ b/sound/firewire/bebob/bebob_pcm.c
@@ -214,7 +214,7 @@ static int pcm_hw_params(struct snd_pcm_substream *substream,
 	struct snd_bebob *bebob = substream->private_data;
 	int err = 0;
 
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+	if (substream->runtime->state == SNDRV_PCM_STATE_OPEN) {
 		unsigned int rate = params_rate(hw_params);
 		unsigned int frames_per_period = params_period_size(hw_params);
 		unsigned int frames_per_buffer = params_buffer_size(hw_params);
@@ -236,7 +236,7 @@ static int pcm_hw_free(struct snd_pcm_substream *substream)
 
 	mutex_lock(&bebob->mutex);
 
-	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+	if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
 		bebob->substreams_counter--;
 
 	snd_bebob_stream_stop_duplex(bebob);
diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
index a69ca1111b03..d64366217d57 100644
--- a/sound/firewire/dice/dice-pcm.c
+++ b/sound/firewire/dice/dice-pcm.c
@@ -266,7 +266,7 @@ static int pcm_hw_params(struct snd_pcm_substream *substream,
 	struct snd_dice *dice = substream->private_data;
 	int err = 0;
 
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+	if (substream->runtime->state == SNDRV_PCM_STATE_OPEN) {
 		unsigned int rate = params_rate(hw_params);
 		unsigned int events_per_period = params_period_size(hw_params);
 		unsigned int events_per_buffer = params_buffer_size(hw_params);
@@ -293,7 +293,7 @@ static int pcm_hw_free(struct snd_pcm_substream *substream)
 
 	mutex_lock(&dice->mutex);
 
-	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+	if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
 		--dice->substreams_counter;
 
 	snd_dice_stream_stop_duplex(dice);
diff --git a/sound/firewire/digi00x/digi00x-pcm.c b/sound/firewire/digi00x/digi00x-pcm.c
index b7f6eda09f9f..3bd1575c9d9c 100644
--- a/sound/firewire/digi00x/digi00x-pcm.c
+++ b/sound/firewire/digi00x/digi00x-pcm.c
@@ -190,7 +190,7 @@ static int pcm_hw_params(struct snd_pcm_substream *substream,
 	struct snd_dg00x *dg00x = substream->private_data;
 	int err = 0;
 
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+	if (substream->runtime->state == SNDRV_PCM_STATE_OPEN) {
 		unsigned int rate = params_rate(hw_params);
 		unsigned int frames_per_period = params_period_size(hw_params);
 		unsigned int frames_per_buffer = params_buffer_size(hw_params);
@@ -212,7 +212,7 @@ static int pcm_hw_free(struct snd_pcm_substream *substream)
 
 	mutex_lock(&dg00x->mutex);
 
-	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+	if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
 		--dg00x->substreams_counter;
 
 	snd_dg00x_stream_stop_duplex(dg00x);
diff --git a/sound/firewire/fireface/ff-pcm.c b/sound/firewire/fireface/ff-pcm.c
index f978cc2fed7d..ec915671a79b 100644
--- a/sound/firewire/fireface/ff-pcm.c
+++ b/sound/firewire/fireface/ff-pcm.c
@@ -230,7 +230,7 @@ static int pcm_hw_params(struct snd_pcm_substream *substream,
 	struct snd_ff *ff = substream->private_data;
 	int err = 0;
 
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+	if (substream->runtime->state == SNDRV_PCM_STATE_OPEN) {
 		unsigned int rate = params_rate(hw_params);
 		unsigned int frames_per_period = params_period_size(hw_params);
 		unsigned int frames_per_buffer = params_buffer_size(hw_params);
@@ -252,7 +252,7 @@ static int pcm_hw_free(struct snd_pcm_substream *substream)
 
 	mutex_lock(&ff->mutex);
 
-	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+	if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
 		--ff->substreams_counter;
 
 	snd_ff_stream_stop_duplex(ff);
diff --git a/sound/firewire/fireworks/fireworks_pcm.c b/sound/firewire/fireworks/fireworks_pcm.c
index a0d5db1d8eb2..c3c21860b245 100644
--- a/sound/firewire/fireworks/fireworks_pcm.c
+++ b/sound/firewire/fireworks/fireworks_pcm.c
@@ -250,7 +250,7 @@ static int pcm_hw_params(struct snd_pcm_substream *substream,
 	struct snd_efw *efw = substream->private_data;
 	int err = 0;
 
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+	if (substream->runtime->state == SNDRV_PCM_STATE_OPEN) {
 		unsigned int rate = params_rate(hw_params);
 		unsigned int frames_per_period = params_period_size(hw_params);
 		unsigned int frames_per_buffer = params_buffer_size(hw_params);
@@ -272,7 +272,7 @@ static int pcm_hw_free(struct snd_pcm_substream *substream)
 
 	mutex_lock(&efw->mutex);
 
-	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+	if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
 		--efw->substreams_counter;
 
 	snd_efw_stream_stop_duplex(efw);
diff --git a/sound/firewire/motu/motu-pcm.c b/sound/firewire/motu/motu-pcm.c
index 8e1437371263..d410c2efbde5 100644
--- a/sound/firewire/motu/motu-pcm.c
+++ b/sound/firewire/motu/motu-pcm.c
@@ -210,7 +210,7 @@ static int pcm_hw_params(struct snd_pcm_substream *substream,
 	struct snd_motu *motu = substream->private_data;
 	int err = 0;
 
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+	if (substream->runtime->state == SNDRV_PCM_STATE_OPEN) {
 		unsigned int rate = params_rate(hw_params);
 		unsigned int frames_per_period = params_period_size(hw_params);
 		unsigned int frames_per_buffer = params_buffer_size(hw_params);
@@ -232,7 +232,7 @@ static int pcm_hw_free(struct snd_pcm_substream *substream)
 
 	mutex_lock(&motu->mutex);
 
-	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+	if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
 		--motu->substreams_counter;
 
 	snd_motu_stream_stop_duplex(motu);
diff --git a/sound/firewire/oxfw/oxfw-pcm.c b/sound/firewire/oxfw/oxfw-pcm.c
index 2dfa7e179cb6..5f43a0b826d2 100644
--- a/sound/firewire/oxfw/oxfw-pcm.c
+++ b/sound/firewire/oxfw/oxfw-pcm.c
@@ -239,7 +239,7 @@ static int pcm_capture_hw_params(struct snd_pcm_substream *substream,
 	struct snd_oxfw *oxfw = substream->private_data;
 	int err = 0;
 
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+	if (substream->runtime->state == SNDRV_PCM_STATE_OPEN) {
 		unsigned int rate = params_rate(hw_params);
 		unsigned int channels = params_channels(hw_params);
 		unsigned int frames_per_period = params_period_size(hw_params);
@@ -262,7 +262,7 @@ static int pcm_playback_hw_params(struct snd_pcm_substream *substream,
 	struct snd_oxfw *oxfw = substream->private_data;
 	int err = 0;
 
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+	if (substream->runtime->state == SNDRV_PCM_STATE_OPEN) {
 		unsigned int rate = params_rate(hw_params);
 		unsigned int channels = params_channels(hw_params);
 		unsigned int frames_per_period = params_period_size(hw_params);
@@ -286,7 +286,7 @@ static int pcm_capture_hw_free(struct snd_pcm_substream *substream)
 
 	mutex_lock(&oxfw->mutex);
 
-	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+	if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
 		--oxfw->substreams_count;
 
 	snd_oxfw_stream_stop_duplex(oxfw);
@@ -301,7 +301,7 @@ static int pcm_playback_hw_free(struct snd_pcm_substream *substream)
 
 	mutex_lock(&oxfw->mutex);
 
-	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+	if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
 		--oxfw->substreams_count;
 
 	snd_oxfw_stream_stop_duplex(oxfw);
diff --git a/sound/firewire/tascam/tascam-pcm.c b/sound/firewire/tascam/tascam-pcm.c
index 36c1353f2494..f6da571707ac 100644
--- a/sound/firewire/tascam/tascam-pcm.c
+++ b/sound/firewire/tascam/tascam-pcm.c
@@ -119,7 +119,7 @@ static int pcm_hw_params(struct snd_pcm_substream *substream,
 	struct snd_tscm *tscm = substream->private_data;
 	int err = 0;
 
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+	if (substream->runtime->state == SNDRV_PCM_STATE_OPEN) {
 		unsigned int rate = params_rate(hw_params);
 		unsigned int frames_per_period = params_period_size(hw_params);
 		unsigned int frames_per_buffer = params_buffer_size(hw_params);
@@ -141,7 +141,7 @@ static int pcm_hw_free(struct snd_pcm_substream *substream)
 
 	mutex_lock(&tscm->mutex);
 
-	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+	if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
 		--tscm->substreams_counter;
 
 	snd_tscm_stream_stop_duplex(tscm);
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 05/11] ALSA: hda: Replace runtime->status->state reference to runtime->state
  2022-09-26 13:55 [PATCH 00/11] ALSA: PCM state reference optimization Takashi Iwai
                   ` (3 preceding siblings ...)
  2022-09-26 13:55 ` [PATCH 04/11] ALSA: firewire: " Takashi Iwai
@ 2022-09-26 13:55 ` Takashi Iwai
  2022-09-26 13:55 ` [PATCH 06/11] ALSA: asihpi: " Takashi Iwai
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2022-09-26 13:55 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai

The recent change in ALSA core allows drivers to get the current PCM
state directly from runtime object.  Replace the calls accordingly.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/hda/hdmi_chmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/hda/hdmi_chmap.c b/sound/hda/hdmi_chmap.c
index aad5c4bf4d34..5d8e1d944b0a 100644
--- a/sound/hda/hdmi_chmap.c
+++ b/sound/hda/hdmi_chmap.c
@@ -774,7 +774,7 @@ static int hdmi_chmap_ctl_put(struct snd_kcontrol *kcontrol,
 	substream = snd_pcm_chmap_substream(info, ctl_idx);
 	if (!substream || !substream->runtime)
 		return 0; /* just for avoiding error from alsactl restore */
-	switch (substream->runtime->status->state) {
+	switch (substream->runtime->state) {
 	case SNDRV_PCM_STATE_OPEN:
 	case SNDRV_PCM_STATE_SETUP:
 		break;
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 06/11] ALSA: asihpi: Replace runtime->status->state reference to runtime->state
  2022-09-26 13:55 [PATCH 00/11] ALSA: PCM state reference optimization Takashi Iwai
                   ` (4 preceding siblings ...)
  2022-09-26 13:55 ` [PATCH 05/11] ALSA: hda: " Takashi Iwai
@ 2022-09-26 13:55 ` Takashi Iwai
  2022-09-26 13:55 ` [PATCH 07/11] ALSA: usb-audio: " Takashi Iwai
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2022-09-26 13:55 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai

The recent change in ALSA core allows drivers to get the current PCM
state directly from runtime object.  Replace the calls accordingly.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/asihpi/asihpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/pci/asihpi/asihpi.c b/sound/pci/asihpi/asihpi.c
index 5e1f9f10051b..8de43aaa10aa 100644
--- a/sound/pci/asihpi/asihpi.c
+++ b/sound/pci/asihpi/asihpi.c
@@ -632,7 +632,7 @@ static int snd_card_asihpi_trigger(struct snd_pcm_substream *substream,
 
 			/*? workaround linked streams don't
 			transition to SETUP 20070706*/
-			s->runtime->status->state = SNDRV_PCM_STATE_SETUP;
+			__snd_pcm_set_state(s->runtime, SNDRV_PCM_STATE_SETUP);
 
 			if (card->support_grouping) {
 				snd_printdd("%d group\n", s->number);
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 07/11] ALSA: usb-audio: Replace runtime->status->state reference to runtime->state
  2022-09-26 13:55 [PATCH 00/11] ALSA: PCM state reference optimization Takashi Iwai
                   ` (5 preceding siblings ...)
  2022-09-26 13:55 ` [PATCH 06/11] ALSA: asihpi: " Takashi Iwai
@ 2022-09-26 13:55 ` Takashi Iwai
  2022-09-26 13:55 ` [PATCH 08/11] ALSA: usx2y: " Takashi Iwai
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2022-09-26 13:55 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai

The recent change in ALSA core allows drivers to get the current PCM
state directly from runtime object.  Replace the calls accordingly.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/pcm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index e721fc12acde..8ed165f036a0 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -1395,7 +1395,7 @@ static int prepare_playback_urb(struct snd_usb_substream *subs,
 	transfer_done = subs->transfer_done;
 
 	if (subs->lowlatency_playback &&
-	    runtime->status->state != SNDRV_PCM_STATE_DRAINING) {
+	    runtime->state != SNDRV_PCM_STATE_DRAINING) {
 		unsigned int hwptr = subs->hwptr_done / stride;
 
 		/* calculate the byte offset-in-buffer of the appl_ptr */
@@ -1583,7 +1583,7 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea
 		return 0;
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_STOP:
-		stop_endpoints(subs, substream->runtime->status->state == SNDRV_PCM_STATE_DRAINING);
+		stop_endpoints(subs, substream->runtime->state == SNDRV_PCM_STATE_DRAINING);
 		snd_usb_endpoint_set_callback(subs->data_endpoint,
 					      NULL, NULL, NULL);
 		subs->running = 0;
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 08/11] ALSA: usx2y: Replace runtime->status->state reference to runtime->state
  2022-09-26 13:55 [PATCH 00/11] ALSA: PCM state reference optimization Takashi Iwai
                   ` (6 preceding siblings ...)
  2022-09-26 13:55 ` [PATCH 07/11] ALSA: usb-audio: " Takashi Iwai
@ 2022-09-26 13:55 ` Takashi Iwai
  2022-09-26 13:55 ` [PATCH 09/11] ASoC: intel: " Takashi Iwai
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2022-09-26 13:55 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai

The recent change in ALSA core allows drivers to get the current PCM
state directly from runtime object.  Replace the calls accordingly.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/usx2y/usbusx2yaudio.c | 3 +--
 sound/usb/usx2y/usx2yhwdeppcm.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c
index 9cd5e3aae4f7..5197599e7aa6 100644
--- a/sound/usb/usx2y/usbusx2yaudio.c
+++ b/sound/usb/usx2y/usbusx2yaudio.c
@@ -822,8 +822,7 @@ static int snd_usx2y_pcm_hw_free(struct snd_pcm_substream *substream)
 		usx2y_urbs_release(subs);
 		if (!cap_subs->pcm_substream ||
 		    !cap_subs->pcm_substream->runtime ||
-		    !cap_subs->pcm_substream->runtime->status ||
-		    cap_subs->pcm_substream->runtime->status->state < SNDRV_PCM_STATE_PREPARED) {
+		    cap_subs->pcm_substream->runtime->state < SNDRV_PCM_STATE_PREPARED) {
 			atomic_set(&cap_subs->state, STATE_STOPPED);
 			usx2y_urbs_release(cap_subs);
 		}
diff --git a/sound/usb/usx2y/usx2yhwdeppcm.c b/sound/usb/usx2y/usx2yhwdeppcm.c
index 240349b644f3..767a227d54da 100644
--- a/sound/usb/usx2y/usx2yhwdeppcm.c
+++ b/sound/usb/usx2y/usx2yhwdeppcm.c
@@ -374,8 +374,7 @@ static int snd_usx2y_usbpcm_hw_free(struct snd_pcm_substream *substream)
 		usx2y_usbpcm_urbs_release(subs);
 		if (!cap_subs->pcm_substream ||
 		    !cap_subs->pcm_substream->runtime ||
-		    !cap_subs->pcm_substream->runtime->status ||
-		    cap_subs->pcm_substream->runtime->status->state < SNDRV_PCM_STATE_PREPARED) {
+		    cap_subs->pcm_substream->runtime->state < SNDRV_PCM_STATE_PREPARED) {
 			atomic_set(&cap_subs->state, STATE_STOPPED);
 			if (cap_subs2)
 				atomic_set(&cap_subs2->state, STATE_STOPPED);
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 09/11] ASoC: intel: Replace runtime->status->state reference to runtime->state
  2022-09-26 13:55 [PATCH 00/11] ALSA: PCM state reference optimization Takashi Iwai
                   ` (7 preceding siblings ...)
  2022-09-26 13:55 ` [PATCH 08/11] ALSA: usx2y: " Takashi Iwai
@ 2022-09-26 13:55 ` Takashi Iwai
  2022-09-26 18:00   ` Mark Brown
  2022-09-26 13:55 ` [PATCH 10/11] ASoC: sh: " Takashi Iwai
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2022-09-26 13:55 UTC (permalink / raw)
  To: alsa-devel
  Cc: Cezary Rojewski, Kai Vehmanen, Takashi Iwai, Bard Liao,
	Pierre-Louis Bossart, Ranjani Sridharan, Liam Girdwood,
	Mark Brown, Peter Ujfalusi

The recent change in ALSA core allows drivers to get the current PCM
state directly from runtime object.  Replace the calls accordingly.

Cc: Cezary Rojewski <cezary.rojewski@intel.com>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Cc: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Cc: Bard Liao <yung-chuan.liao@linux.intel.com>
Cc: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Cc: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---

I'm going to merge this through my sound git tree as it requires ALSA
core changes.

 sound/soc/intel/skylake/skl-pcm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 9d72ebd812af..1015716f9336 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -275,7 +275,7 @@ static int skl_pcm_prepare(struct snd_pcm_substream *substream,
 	 * calls prepare another time, reset the FW pipe to clean state
 	 */
 	if (mconfig &&
-		(substream->runtime->status->state == SNDRV_PCM_STATE_XRUN ||
+		(substream->runtime->state == SNDRV_PCM_STATE_XRUN ||
 		 mconfig->pipe->state == SKL_PIPE_CREATED ||
 		 mconfig->pipe->state == SKL_PIPE_PAUSED)) {
 
@@ -593,7 +593,7 @@ static int skl_link_pcm_prepare(struct snd_pcm_substream *substream,
 	/* In case of XRUN recovery, reset the FW pipe to clean state */
 	mconfig = skl_tplg_be_get_cpr_module(dai, substream->stream);
 	if (mconfig && !mconfig->pipe->passthru &&
-		(substream->runtime->status->state == SNDRV_PCM_STATE_XRUN))
+		(substream->runtime->state == SNDRV_PCM_STATE_XRUN))
 		skl_reset_pipe(skl, mconfig->pipe);
 
 	return 0;
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 10/11] ASoC: sh: Replace runtime->status->state reference to runtime->state
  2022-09-26 13:55 [PATCH 00/11] ALSA: PCM state reference optimization Takashi Iwai
                   ` (8 preceding siblings ...)
  2022-09-26 13:55 ` [PATCH 09/11] ASoC: intel: " Takashi Iwai
@ 2022-09-26 13:55 ` Takashi Iwai
  2022-09-26 18:01   ` Mark Brown
  2022-09-26 13:55 ` [PATCH 11/11] usb: gadget: " Takashi Iwai
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2022-09-26 13:55 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai, Mark Brown

The recent change in ALSA core allows drivers to get the current PCM
state directly from runtime object.  Replace the calls accordingly.

Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---

I'm going to merge this through my sound git tree as it requires ALSA
core changes.

 sound/soc/sh/rz-ssi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c
index 7ace0c0db5b1..5d6bae33ae34 100644
--- a/sound/soc/sh/rz-ssi.c
+++ b/sound/soc/sh/rz-ssi.c
@@ -598,7 +598,7 @@ static int rz_ssi_dma_transfer(struct rz_ssi_priv *ssi,
 		return -EINVAL;
 
 	runtime = substream->runtime;
-	if (runtime->status->state == SNDRV_PCM_STATE_DRAINING)
+	if (runtime->state == SNDRV_PCM_STATE_DRAINING)
 		/*
 		 * Stream is ending, so do not queue up any more DMA
 		 * transfers otherwise we play partial sound clips
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 11/11] usb: gadget: Replace runtime->status->state reference to runtime->state
  2022-09-26 13:55 [PATCH 00/11] ALSA: PCM state reference optimization Takashi Iwai
                   ` (9 preceding siblings ...)
  2022-09-26 13:55 ` [PATCH 10/11] ASoC: sh: " Takashi Iwai
@ 2022-09-26 13:55 ` Takashi Iwai
  2022-09-26 14:05   ` Greg Kroah-Hartman
  2022-09-26 15:56 ` [PATCH 00/11] ALSA: PCM state reference optimization Jaroslav Kysela
  2022-09-27  1:22 ` Takashi Sakamoto
  12 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2022-09-26 13:55 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai, Felipe Balbi, Greg Kroah-Hartman

The recent change in ALSA core allows drivers to get the current PCM
state directly from runtime object.  Replace the calls accordingly.

Cc: Felipe Balbi <balbi@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---

I'm going to merge this through sound git tree as it requires ALSA core
changes.

 drivers/usb/gadget/function/u_uac1_legacy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/u_uac1_legacy.c b/drivers/usb/gadget/function/u_uac1_legacy.c
index 60ae8b2d3f6a..dd21c251542c 100644
--- a/drivers/usb/gadget/function/u_uac1_legacy.c
+++ b/drivers/usb/gadget/function/u_uac1_legacy.c
@@ -158,8 +158,8 @@ size_t u_audio_playback(struct gaudio *card, void *buf, size_t count)
 	snd_pcm_sframes_t frames;
 
 try_again:
-	if (runtime->status->state == SNDRV_PCM_STATE_XRUN ||
-		runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) {
+	if (runtime->state == SNDRV_PCM_STATE_XRUN ||
+		runtime->state == SNDRV_PCM_STATE_SUSPENDED) {
 		result = snd_pcm_kernel_ioctl(substream,
 				SNDRV_PCM_IOCTL_PREPARE, NULL);
 		if (result < 0) {
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 11/11] usb: gadget: Replace runtime->status->state reference to runtime->state
  2022-09-26 13:55 ` [PATCH 11/11] usb: gadget: " Takashi Iwai
@ 2022-09-26 14:05   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 20+ messages in thread
From: Greg Kroah-Hartman @ 2022-09-26 14:05 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Felipe Balbi, alsa-devel

On Mon, Sep 26, 2022 at 03:55:58PM +0200, Takashi Iwai wrote:
> The recent change in ALSA core allows drivers to get the current PCM
> state directly from runtime object.  Replace the calls accordingly.
> 
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
> 
> I'm going to merge this through sound git tree as it requires ALSA core
> changes.

Looks good to me:

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 00/11] ALSA: PCM state reference optimization
  2022-09-26 13:55 [PATCH 00/11] ALSA: PCM state reference optimization Takashi Iwai
                   ` (10 preceding siblings ...)
  2022-09-26 13:55 ` [PATCH 11/11] usb: gadget: " Takashi Iwai
@ 2022-09-26 15:56 ` Jaroslav Kysela
  2022-09-26 16:05   ` Takashi Iwai
  2022-09-27  1:22 ` Takashi Sakamoto
  12 siblings, 1 reply; 20+ messages in thread
From: Jaroslav Kysela @ 2022-09-26 15:56 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: ALSA development

On 26. 09. 22 15:55, Takashi Iwai wrote:
> Hi,
> 
> this is a patch set for simplifying the reference to the current PCM
> state by having the local copy in runtime instead of relying on
> runtime->status indirection.  This also hardens against the attack by
> modifying the mmapped status record.
> 
> The first patch does the basic job in the core PCM side, and the
> second patch flips the PCM status mmap to read-only for hardening,
> while the remaining patches are for drivers to follow the core
> change.
> 
> The conversions are straightforward.  In most places, it's just
> replacing runtime->status->state with runtime->state.
> 
> 
> Takashi
> 
> ===
> 
> Takashi Iwai (11):
>    ALSA: pcm: Avoid reference to status->state
>    ALSA: pcm: Make mmap status read-only
>    ALSA: aloop: Replace runtime->status->state reference to
>      runtime->state
>    ALSA: firewire: Replace runtime->status->state reference to
>      runtime->state
>    ALSA: hda: Replace runtime->status->state reference to runtime->state
>    ALSA: asihpi: Replace runtime->status->state reference to
>      runtime->state
>    ALSA: usb-audio: Replace runtime->status->state reference to
>      runtime->state
>    ALSA: usx2y: Replace runtime->status->state reference to
>      runtime->state
>    ASoC: intel: Replace runtime->status->state reference to
>      runtime->state
>    ASoC: sh: Replace runtime->status->state reference to runtime->state
>    usb: gadget: Replace runtime->status->state reference to
>      runtime->state

Nice cleanup. Perhaps, you may add a note to the second patch that the status 
is already mmaped as read-only in alsa-lib for ages. So no regressions are 
expected.

Reviewed-by: Jaroslav Kysela <perex@perex.cz>

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 00/11] ALSA: PCM state reference optimization
  2022-09-26 15:56 ` [PATCH 00/11] ALSA: PCM state reference optimization Jaroslav Kysela
@ 2022-09-26 16:05   ` Takashi Iwai
  0 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2022-09-26 16:05 UTC (permalink / raw)
  To: Jaroslav Kysela; +Cc: ALSA development

On Mon, 26 Sep 2022 17:56:15 +0200,
Jaroslav Kysela wrote:
> 
> On 26. 09. 22 15:55, Takashi Iwai wrote:
> > Hi,
> > 
> > this is a patch set for simplifying the reference to the current PCM
> > state by having the local copy in runtime instead of relying on
> > runtime->status indirection.  This also hardens against the attack by
> > modifying the mmapped status record.
> > 
> > The first patch does the basic job in the core PCM side, and the
> > second patch flips the PCM status mmap to read-only for hardening,
> > while the remaining patches are for drivers to follow the core
> > change.
> > 
> > The conversions are straightforward.  In most places, it's just
> > replacing runtime->status->state with runtime->state.
> > 
> > 
> > Takashi
> > 
> > ===
> > 
> > Takashi Iwai (11):
> >    ALSA: pcm: Avoid reference to status->state
> >    ALSA: pcm: Make mmap status read-only
> >    ALSA: aloop: Replace runtime->status->state reference to
> >      runtime->state
> >    ALSA: firewire: Replace runtime->status->state reference to
> >      runtime->state
> >    ALSA: hda: Replace runtime->status->state reference to runtime->state
> >    ALSA: asihpi: Replace runtime->status->state reference to
> >      runtime->state
> >    ALSA: usb-audio: Replace runtime->status->state reference to
> >      runtime->state
> >    ALSA: usx2y: Replace runtime->status->state reference to
> >      runtime->state
> >    ASoC: intel: Replace runtime->status->state reference to
> >      runtime->state
> >    ASoC: sh: Replace runtime->status->state reference to runtime->state
> >    usb: gadget: Replace runtime->status->state reference to
> >      runtime->state
> 
> Nice cleanup. Perhaps, you may add a note to the second patch that the
> status is already mmaped as read-only in alsa-lib for ages. So no
> regressions are expected.

Makes sense.  Will add some text.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 09/11] ASoC: intel: Replace runtime->status->state reference to runtime->state
  2022-09-26 13:55 ` [PATCH 09/11] ASoC: intel: " Takashi Iwai
@ 2022-09-26 18:00   ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2022-09-26 18:00 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, Kai Vehmanen, Peter Ujfalusi, Cezary Rojewski,
	Pierre-Louis Bossart, Liam Girdwood, Ranjani Sridharan,
	Bard Liao

[-- Attachment #1: Type: text/plain, Size: 246 bytes --]

On Mon, Sep 26, 2022 at 03:55:56PM +0200, Takashi Iwai wrote:
> The recent change in ALSA core allows drivers to get the current PCM
> state directly from runtime object.  Replace the calls accordingly.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 10/11] ASoC: sh: Replace runtime->status->state reference to runtime->state
  2022-09-26 13:55 ` [PATCH 10/11] ASoC: sh: " Takashi Iwai
@ 2022-09-26 18:01   ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2022-09-26 18:01 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

[-- Attachment #1: Type: text/plain, Size: 246 bytes --]

On Mon, Sep 26, 2022 at 03:55:57PM +0200, Takashi Iwai wrote:
> The recent change in ALSA core allows drivers to get the current PCM
> state directly from runtime object.  Replace the calls accordingly.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 00/11] ALSA: PCM state reference optimization
  2022-09-26 13:55 [PATCH 00/11] ALSA: PCM state reference optimization Takashi Iwai
                   ` (11 preceding siblings ...)
  2022-09-26 15:56 ` [PATCH 00/11] ALSA: PCM state reference optimization Jaroslav Kysela
@ 2022-09-27  1:22 ` Takashi Sakamoto
  2022-09-27  6:26   ` Takashi Iwai
  12 siblings, 1 reply; 20+ messages in thread
From: Takashi Sakamoto @ 2022-09-27  1:22 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Felipe Balbi, alsa-devel, Kai Vehmanen, Greg Kroah-Hartman,
	Peter Ujfalusi, Cezary Rojewski, Pierre-Louis Bossart,
	Liam Girdwood, Mark Brown, Ranjani Sridharan, Bard Liao

Hi,

On Mon, Sep 26, 2022 at 03:55:47PM +0200, Takashi Iwai wrote:
> Hi,
> 
> this is a patch set for simplifying the reference to the current PCM
> state by having the local copy in runtime instead of relying on
> runtime->status indirection.  This also hardens against the attack by
> modifying the mmapped status record.
 
The overall patches looks good to me and I have no objections, while I
have some slight opinions to them in a place of sound driver developer.

> The first patch does the basic job in the core PCM side,

The main concern is indirect accessing to state field via some pointer
hops. I think addition of helper macro at first step eases centre of your
work, like:

```
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 8c48a5bce88c..f6a160cb8135 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -669,6 +669,20 @@ void snd_pcm_stream_unlock_irqrestore(struct snd_pcm_substream *substream,
             stream <= SNDRV_PCM_STREAM_LAST;           \
             stream++)
 
+/**
+ * snd_pcm_stream_state - Return state in runtime of the PCM substream.
+ * @substream: substream to check. runtime should be attached.
+ *
+ * Return state in runtime of the PCM substream. The substream should exists and
+ * runtime should be attached to it.
+ */
+static inline snd_pcm_state_t snd_pcm_stream_state(const snd_pcm_substream *substream)
+{
+       snd_BUG_ON(!(sub) || !(sub)->runtime);
+
+       return substream->runtime->status->state;
+}
```

As we can see, sound driver programmer sometimes checks state of runtime
in their code, thus the macro could helps them as well as centre of your
change.

> and the
> second patch flips the PCM status mmap to read-only for hardening,

The patch looks good to me as well as the patch for asihpi driver, and
should be in your tree independent from the others.

> while the remaining patches are for drivers to follow the core
> change.
>
> The conversions are straightforward.  In most places, it's just
> replacing runtime->status->state with runtime->state.

The usage of mentioned macro can control the concerned access. Now
addition of new field, `state` to runtime structure can be done easily.


> Takashi
> 
> ===
> 
> Takashi Iwai (11):
>   ALSA: pcm: Avoid reference to status->state
>   ALSA: pcm: Make mmap status read-only
>   ALSA: aloop: Replace runtime->status->state reference to
>     runtime->state
>   ALSA: firewire: Replace runtime->status->state reference to
>     runtime->state
>   ALSA: hda: Replace runtime->status->state reference to runtime->state
>   ALSA: asihpi: Replace runtime->status->state reference to
>     runtime->state
>   ALSA: usb-audio: Replace runtime->status->state reference to
>     runtime->state
>   ALSA: usx2y: Replace runtime->status->state reference to
>     runtime->state
>   ASoC: intel: Replace runtime->status->state reference to
>     runtime->state
>   ASoC: sh: Replace runtime->status->state reference to runtime->state
>   usb: gadget: Replace runtime->status->state reference to
>     runtime->state
> 
>  drivers/usb/gadget/function/u_uac1_legacy.c |   4 +-
>  include/sound/pcm.h                         |  20 ++-
>  sound/core/oss/pcm_oss.c                    |  42 +++----
>  sound/core/pcm.c                            |   9 +-
>  sound/core/pcm_compat.c                     |   4 +-
>  sound/core/pcm_lib.c                        |  16 +--
>  sound/core/pcm_native.c                     | 128 ++++++++++----------
>  sound/drivers/aloop.c                       |   4 +-
>  sound/firewire/bebob/bebob_pcm.c            |   4 +-
>  sound/firewire/dice/dice-pcm.c              |   4 +-
>  sound/firewire/digi00x/digi00x-pcm.c        |   4 +-
>  sound/firewire/fireface/ff-pcm.c            |   4 +-
>  sound/firewire/fireworks/fireworks_pcm.c    |   4 +-
>  sound/firewire/motu/motu-pcm.c              |   4 +-
>  sound/firewire/oxfw/oxfw-pcm.c              |   8 +-
>  sound/firewire/tascam/tascam-pcm.c          |   4 +-
>  sound/hda/hdmi_chmap.c                      |   2 +-
>  sound/pci/asihpi/asihpi.c                   |   2 +-
>  sound/soc/intel/skylake/skl-pcm.c           |   4 +-
>  sound/soc/sh/rz-ssi.c                       |   2 +-
>  sound/usb/pcm.c                             |   4 +-
>  sound/usb/usx2y/usbusx2yaudio.c             |   3 +-
>  sound/usb/usx2y/usx2yhwdeppcm.c             |   3 +-
>  23 files changed, 150 insertions(+), 133 deletions(-)
> 
> ===
> 
> Cc: Bard Liao <yung-chuan.liao@linux.intel.com>
> Cc: Cezary Rojewski <cezary.rojewski@intel.com>
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> Cc: Liam Girdwood <liam.r.girdwood@linux.intel.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Cc: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Cc: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> 
> -- 
> 2.35.3

Regards

Takashi Sakamoto

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 00/11] ALSA: PCM state reference optimization
  2022-09-27  1:22 ` Takashi Sakamoto
@ 2022-09-27  6:26   ` Takashi Iwai
  2022-09-27 14:25     ` Takashi Sakamoto
  0 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2022-09-27  6:26 UTC (permalink / raw)
  To: Takashi Sakamoto
  Cc: Felipe Balbi, alsa-devel, Kai Vehmanen, Greg Kroah-Hartman,
	Peter Ujfalusi, Cezary Rojewski, Pierre-Louis Bossart,
	Liam Girdwood, Mark Brown, Ranjani Sridharan, Bard Liao

On Tue, 27 Sep 2022 03:22:07 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On Mon, Sep 26, 2022 at 03:55:47PM +0200, Takashi Iwai wrote:
> > Hi,
> > 
> > this is a patch set for simplifying the reference to the current PCM
> > state by having the local copy in runtime instead of relying on
> > runtime->status indirection.  This also hardens against the attack by
> > modifying the mmapped status record.
>  
> The overall patches looks good to me and I have no objections, while I
> have some slight opinions to them in a place of sound driver developer.
> 
> > The first patch does the basic job in the core PCM side,
> 
> The main concern is indirect accessing to state field via some pointer
> hops. I think addition of helper macro at first step eases centre of your
> work, like:
> 
> ```
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index 8c48a5bce88c..f6a160cb8135 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -669,6 +669,20 @@ void snd_pcm_stream_unlock_irqrestore(struct snd_pcm_substream *substream,
>              stream <= SNDRV_PCM_STREAM_LAST;           \
>              stream++)
>  
> +/**
> + * snd_pcm_stream_state - Return state in runtime of the PCM substream.
> + * @substream: substream to check. runtime should be attached.
> + *
> + * Return state in runtime of the PCM substream. The substream should exists and
> + * runtime should be attached to it.
> + */
> +static inline snd_pcm_state_t snd_pcm_stream_state(const snd_pcm_substream *substream)
> +{
> +       snd_BUG_ON(!(sub) || !(sub)->runtime);
> +
> +       return substream->runtime->status->state;
> +}
> ```
> 
> As we can see, sound driver programmer sometimes checks state of runtime
> in their code, thus the macro could helps them as well as centre of your
> change.

This might help if we may need any change in future again.

But the NULL check is superfluous in most places, and the state check
is done often in a hot path, it's better to rip off.  So it's only
about the read of runtime->state, and then it's doubtful whether it
brings much value at this moment; that's the reason I didn't introduce
such a macro for now.


Takashi

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 00/11] ALSA: PCM state reference optimization
  2022-09-27  6:26   ` Takashi Iwai
@ 2022-09-27 14:25     ` Takashi Sakamoto
  0 siblings, 0 replies; 20+ messages in thread
From: Takashi Sakamoto @ 2022-09-27 14:25 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Felipe Balbi, alsa-devel, Kai Vehmanen, Greg Kroah-Hartman,
	Peter Ujfalusi, Cezary Rojewski, Pierre-Louis Bossart,
	Liam Girdwood, Mark Brown, Ranjani Sridharan, Bard Liao

On Tue, Sep 27, 2022 at 08:26:11AM +0200, Takashi Iwai wrote:
> On Tue, 27 Sep 2022 03:22:07 +0200,
> Takashi Sakamoto wrote:
> > 
> > Hi,
> > 
> > On Mon, Sep 26, 2022 at 03:55:47PM +0200, Takashi Iwai wrote:
> > > Hi,
> > > 
> > > this is a patch set for simplifying the reference to the current PCM
> > > state by having the local copy in runtime instead of relying on
> > > runtime->status indirection.  This also hardens against the attack by
> > > modifying the mmapped status record.
> >  
> > The overall patches looks good to me and I have no objections, while I
> > have some slight opinions to them in a place of sound driver developer.
> > 
> > > The first patch does the basic job in the core PCM side,
> > 
> > The main concern is indirect accessing to state field via some pointer
> > hops. I think addition of helper macro at first step eases centre of your
> > work, like:
> > 
> > ```
> > diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> > index 8c48a5bce88c..f6a160cb8135 100644
> > --- a/include/sound/pcm.h
> > +++ b/include/sound/pcm.h
> > @@ -669,6 +669,20 @@ void snd_pcm_stream_unlock_irqrestore(struct snd_pcm_substream *substream,
> >              stream <= SNDRV_PCM_STREAM_LAST;           \
> >              stream++)
> >  
> > +/**
> > + * snd_pcm_stream_state - Return state in runtime of the PCM substream.
> > + * @substream: substream to check. runtime should be attached.
> > + *
> > + * Return state in runtime of the PCM substream. The substream should exists and
> > + * runtime should be attached to it.
> > + */
> > +static inline snd_pcm_state_t snd_pcm_stream_state(const snd_pcm_substream *substream)
> > +{
> > +       snd_BUG_ON(!(sub) || !(sub)->runtime);
> > +
> > +       return substream->runtime->status->state;
> > +}
> > ```
> > 
> > As we can see, sound driver programmer sometimes checks state of runtime
> > in their code, thus the macro could helps them as well as centre of your
> > change.
> 
> This might help if we may need any change in future again.
> 
> But the NULL check is superfluous in most places, and the state check
> is done often in a hot path, it's better to rip off.  So it's only
> about the read of runtime->state, and then it's doubtful whether it
> brings much value at this moment; that's the reason I didn't introduce
> such a macro for now.

Yes, so I have no strong suggestion to introduce the macro and no
objections to apply the patches as is.

I'm too conservative developer and consider about usage of memory
barrier always when seeing such state check since we should pay enough
attension to instruction reordering and concurrent accesses across
processors for such case. Fortunately we have no reports for such issue
yet. When posting the last message, it was on my mind and it's still.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2022-09-27 14:26 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-26 13:55 [PATCH 00/11] ALSA: PCM state reference optimization Takashi Iwai
2022-09-26 13:55 ` [PATCH 01/11] ALSA: pcm: Avoid reference to status->state Takashi Iwai
2022-09-26 13:55 ` [PATCH 02/11] ALSA: pcm: Make mmap status read-only Takashi Iwai
2022-09-26 13:55 ` [PATCH 03/11] ALSA: aloop: Replace runtime->status->state reference to runtime->state Takashi Iwai
2022-09-26 13:55 ` [PATCH 04/11] ALSA: firewire: " Takashi Iwai
2022-09-26 13:55 ` [PATCH 05/11] ALSA: hda: " Takashi Iwai
2022-09-26 13:55 ` [PATCH 06/11] ALSA: asihpi: " Takashi Iwai
2022-09-26 13:55 ` [PATCH 07/11] ALSA: usb-audio: " Takashi Iwai
2022-09-26 13:55 ` [PATCH 08/11] ALSA: usx2y: " Takashi Iwai
2022-09-26 13:55 ` [PATCH 09/11] ASoC: intel: " Takashi Iwai
2022-09-26 18:00   ` Mark Brown
2022-09-26 13:55 ` [PATCH 10/11] ASoC: sh: " Takashi Iwai
2022-09-26 18:01   ` Mark Brown
2022-09-26 13:55 ` [PATCH 11/11] usb: gadget: " Takashi Iwai
2022-09-26 14:05   ` Greg Kroah-Hartman
2022-09-26 15:56 ` [PATCH 00/11] ALSA: PCM state reference optimization Jaroslav Kysela
2022-09-26 16:05   ` Takashi Iwai
2022-09-27  1:22 ` Takashi Sakamoto
2022-09-27  6:26   ` Takashi Iwai
2022-09-27 14:25     ` Takashi Sakamoto

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.