[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <79b86a0618328ba1d0cb5cf4011fd73ac6900e8f.1749724478.git.christophe.leroy@csgroup.eu>
Date: Thu, 12 Jun 2025 12:51:05 +0200
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Jaroslav Kysela <perex@...ex.cz>,
Takashi Iwai <tiwai@...e.com>
Cc: Christophe Leroy <christophe.leroy@...roup.eu>,
linux-kernel@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org,
linux-sound@...r.kernel.org,
Herve Codina <herve.codina@...tlin.com>,
Mark Brown <broonie@...nel.org>
Subject: [RFC PATCH 3/3] ALSA: pcm: Convert snd_pcm_sync_ptr() to user_access_begin/user_access_end()
Now that snd_pcm_sync_ptr_get_user() and snd_pcm_sync_ptr_put_user()
are converted to user_access_begin/user_access_end(),
snd_pcm_sync_ptr_get_user() is more efficient than a raw get_user()
followed by a copy_from_user(). And because copy_{to/from}_user() are
generic functions focussed on transfer of big data blocks to/from user,
snd_pcm_sync_ptr_put_user() is also more efficient for small amont of
data.
So use snd_pcm_sync_ptr_get_user() and snd_pcm_sync_ptr_put_user() in
snd_pcm_sync_ptr() too.
In order to have snd_pcm_mmap_status32 similar to snd_pcm_mmap_status,
replace to tsamp_{sec/nsec} and audio_tstamp_{sec/nsec} by equivalent
struct __snd_timespec.
snd_pcm_ioctl_sync_ptr_buggy() is left as it is because the conversion
wouldn't be straigh-forward do to the workaround it provides.
Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
---
sound/core/pcm_native.c | 52 +++++++++++++++++++----------------------
1 file changed, 24 insertions(+), 28 deletions(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 554352f546c9..31ddfdb0edfa 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -3077,11 +3077,11 @@ failed: \
goto failed; \
unsafe_put_user(__s.state, &__src->s.status.state, failed); \
unsafe_put_user(__s.hw_ptr, &__src->s.status.hw_ptr, failed); \
- unsafe_put_user(__s.tstamp.tv_sec, &__src->s.status.tstamp_sec, failed);\
- unsafe_put_user(__s.tstamp.tv_nsec, &__src->s.status.tstamp_nsec, failed); \
+ unsafe_put_user(__s.tstamp.tv_sec, &__src->s.status.tstamp.tv_sec, failed); \
+ unsafe_put_user(__s.tstamp.tv_nsec, &__src->s.status.tstamp.tv_nsec, failed); \
unsafe_put_user(__s.suspended_state, &__src->s.status.suspended_state, failed); \
- unsafe_put_user(__s.audio_tstamp.tv_sec, &__src->s.status.audio_tstamp_sec, failed); \
- unsafe_put_user(__s.audio_tstamp.tv_nsec, &__src->s.status.audio_tstamp_nsec, failed); \
+ unsafe_put_user(__s.audio_tstamp.tv_sec, &__src->s.status.audio_tstamp.tv_sec, failed); \
+ unsafe_put_user(__s.audio_tstamp.tv_nsec, &__src->s.status.audio_tstamp.tv_nsec, failed);\
unsafe_put_user(__c.appl_ptr, &__src->c.control.appl_ptr, failed); \
unsafe_put_user(__c.avail_min, &__src->c.control.avail_min, failed); \
err = 0; \
@@ -3094,45 +3094,43 @@ static int snd_pcm_sync_ptr(struct snd_pcm_substream *substream,
struct snd_pcm_sync_ptr __user *_sync_ptr)
{
struct snd_pcm_runtime *runtime = substream->runtime;
- struct snd_pcm_sync_ptr sync_ptr;
volatile struct snd_pcm_mmap_status *status;
volatile struct snd_pcm_mmap_control *control;
+ u32 sflags;
+ struct snd_pcm_mmap_control scontrol;
+ struct snd_pcm_mmap_status sstatus;
int err;
- memset(&sync_ptr, 0, sizeof(sync_ptr));
- if (get_user(sync_ptr.flags, (unsigned __user *)&(_sync_ptr->flags)))
+ if (snd_pcm_sync_ptr_get_user(sflags, scontrol, _sync_ptr))
return -EFAULT;
- if (copy_from_user(&sync_ptr.c.control, &(_sync_ptr->c.control), sizeof(struct snd_pcm_mmap_control)))
- return -EFAULT;
status = runtime->status;
control = runtime->control;
- if (sync_ptr.flags & SNDRV_PCM_SYNC_PTR_HWSYNC) {
+ if (sflags & SNDRV_PCM_SYNC_PTR_HWSYNC) {
err = snd_pcm_hwsync(substream);
if (err < 0)
return err;
}
scoped_guard(pcm_stream_lock_irq, substream) {
- if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_APPL)) {
- err = pcm_lib_apply_appl_ptr(substream,
- sync_ptr.c.control.appl_ptr);
+ if (!(sflags & SNDRV_PCM_SYNC_PTR_APPL)) {
+ err = pcm_lib_apply_appl_ptr(substream, scontrol.appl_ptr);
if (err < 0)
return err;
} else {
- sync_ptr.c.control.appl_ptr = control->appl_ptr;
+ scontrol.appl_ptr = control->appl_ptr;
}
- if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_AVAIL_MIN))
- control->avail_min = sync_ptr.c.control.avail_min;
+ if (!(sflags & SNDRV_PCM_SYNC_PTR_AVAIL_MIN))
+ control->avail_min = scontrol.avail_min;
else
- sync_ptr.c.control.avail_min = control->avail_min;
- sync_ptr.s.status.state = status->state;
- sync_ptr.s.status.hw_ptr = status->hw_ptr;
- sync_ptr.s.status.tstamp = status->tstamp;
- sync_ptr.s.status.suspended_state = status->suspended_state;
- sync_ptr.s.status.audio_tstamp = status->audio_tstamp;
+ scontrol.avail_min = control->avail_min;
+ sstatus.state = status->state;
+ sstatus.hw_ptr = status->hw_ptr;
+ sstatus.tstamp = status->tstamp;
+ sstatus.suspended_state = status->suspended_state;
+ sstatus.audio_tstamp = status->audio_tstamp;
}
- if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_APPL))
+ if (!(sflags & SNDRV_PCM_SYNC_PTR_APPL))
snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_DEVICE);
- if (copy_to_user(_sync_ptr, &sync_ptr, sizeof(sync_ptr)))
+ if (snd_pcm_sync_ptr_put_user(sstatus, scontrol, _sync_ptr))
return -EFAULT;
return 0;
}
@@ -3141,11 +3139,9 @@ struct snd_pcm_mmap_status32 {
snd_pcm_state_t state;
s32 pad1;
u32 hw_ptr;
- s32 tstamp_sec;
- s32 tstamp_nsec;
+ struct __snd_timespec tstamp;
snd_pcm_state_t suspended_state;
- s32 audio_tstamp_sec;
- s32 audio_tstamp_nsec;
+ struct __snd_timespec audio_tstamp;
} __packed;
struct snd_pcm_mmap_control32 {
--
2.47.0
Powered by blists - more mailing lists