[alsa-devel] [PATCH 0/4] ALSA: Fix year 2038 issue for sound subsystem, alternative
I've tried the suggestion from Jaroslaw, doing a minimal change to the UAPI headers to keep the existing binary interface. As he predicted, this is a much simpler set of kernel changes, but we will pay for that with added complexity in alsa-lib.
The first two patches in this series are taken from Baolin's patch set, with a small bugfix folded in to avoid a compile-time regression.
The other two patches are to redefine the UAPI and to deprecate the support for CLOCK_REALTIME time stamps, which we can no longer allow with user space that we expect to survive beyond 2038.
Overall, I'd still be happier with Baolin's approach since it allows us to keep compatiblity with CLOCK_REALTIME users and requires fewer changes in user space, but this would work as well.
Arnd
Cc: Jaroslav Kysela perex@perex.cz Cc: tiwai@suse.com Cc: lgirdwood@gmail.com Cc: broonie@kernel.org Cc: o-takashi@sakamocchi.jp Cc: y2038@lists.linaro.org Cc: alsa-devel@alsa-project.org Cc: linux-kernel@vger.kernel.org Cc: Baolin Wang baolin.wang@linaro.org
Arnd Bergmann (2): ALSA: replace timespec types in uapi headers ALSA: Deprecate CLOCK_REALTIME timestamps
Baolin Wang (2): ALSA: Replace timespec with timespec64 ALSA: Avoid using timespec for struct snd_ctl_elem_value
include/sound/asound.h | 8 +++++ include/sound/pcm.h | 22 +++++++----- include/sound/timer.h | 4 +-- include/uapi/sound/asound.h | 53 +++++++++++++++++++++-------- sound/core/Kconfig | 11 ++++++ sound/core/compat.h | 11 ++++++ sound/core/pcm.c | 3 ++ sound/core/pcm_compat.c | 70 ++++++++++++++++++++++----------------- sound/core/pcm_lib.c | 36 ++++++++++++-------- sound/core/pcm_native.c | 25 ++++++++++---- sound/core/rawmidi_compat.c | 12 +++---- sound/core/timer.c | 28 ++++++++-------- sound/core/timer_compat.c | 4 ++- sound/pci/hda/hda_controller.c | 14 +++++--- sound/soc/intel/skylake/skl-pcm.c | 4 +-- 15 files changed, 203 insertions(+), 102 deletions(-) create mode 100644 sound/core/compat.h
From: Baolin Wang baolin.wang@linaro.org
Since timespec is not year 2038 safe on 32bit system, and we need to convert all timespec variables to timespec64 type for sound subsystem.
This patch is used to do preparation for following patches, that will convert all structures defined in uapi/sound/asound.h to use 64-bit time_t.
Signed-off-by: Baolin Wang baolin.wang@linaro.org [arnd: fix a compile-time bug] Signed-off-by: Arnd Bergmann arnd@arndb.de --- include/sound/pcm.h | 18 +++++++++--------- include/sound/timer.h | 4 ++-- sound/core/pcm_compat.c | 32 ++++++++++++++++++++------------ sound/core/pcm_lib.c | 36 ++++++++++++++++++++++-------------- sound/core/pcm_native.c | 12 ++++++++---- sound/core/timer.c | 28 ++++++++++++++-------------- sound/pci/hda/hda_controller.c | 10 +++++----- sound/soc/intel/skylake/skl-pcm.c | 4 ++-- 8 files changed, 82 insertions(+), 62 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index e054c583d3b3..973d6745217a 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -75,7 +75,7 @@ struct snd_pcm_ops { int (*trigger)(struct snd_pcm_substream *substream, int cmd); snd_pcm_uframes_t (*pointer)(struct snd_pcm_substream *substream); int (*get_time_info)(struct snd_pcm_substream *substream, - struct timespec *system_ts, struct timespec *audio_ts, + struct timespec64 *system_ts, struct timespec64 *audio_ts, struct snd_pcm_audio_tstamp_config *audio_tstamp_config, struct snd_pcm_audio_tstamp_report *audio_tstamp_report); int (*fill_silence)(struct snd_pcm_substream *substream, int channel, @@ -351,7 +351,7 @@ static inline void snd_pcm_pack_audio_tstamp_report(__u32 *data, __u32 *accuracy struct snd_pcm_runtime { /* -- Status -- */ struct snd_pcm_substream *trigger_master; - struct timespec trigger_tstamp; /* trigger timestamp */ + struct timespec64 trigger_tstamp; /* trigger timestamp */ bool trigger_tstamp_latched; /* trigger timestamp latched in low-level driver/hardware */ int overrange; snd_pcm_uframes_t avail_max; @@ -427,7 +427,7 @@ struct snd_pcm_runtime { /* -- audio timestamp config -- */ struct snd_pcm_audio_tstamp_config audio_tstamp_config; struct snd_pcm_audio_tstamp_report audio_tstamp_report; - struct timespec driver_tstamp; + struct timespec64 driver_tstamp;
#if IS_ENABLED(CONFIG_SND_PCM_OSS) /* -- OSS things -- */ @@ -1175,22 +1175,22 @@ static inline void snd_pcm_set_runtime_buffer(struct snd_pcm_substream *substrea }
/** - * snd_pcm_gettime - Fill the timespec depending on the timestamp mode + * snd_pcm_gettime - Fill the timespec64 depending on the timestamp mode * @runtime: PCM runtime instance - * @tv: timespec to fill + * @tv: timespec64 to fill */ static inline void snd_pcm_gettime(struct snd_pcm_runtime *runtime, - struct timespec *tv) + struct timespec64 *tv) { switch (runtime->tstamp_type) { case SNDRV_PCM_TSTAMP_TYPE_MONOTONIC: - ktime_get_ts(tv); + ktime_get_ts64(tv); break; case SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW: - getrawmonotonic(tv); + getrawmonotonic64(tv); break; default: - getnstimeofday(tv); + ktime_get_real_ts64(tv); break; } } diff --git a/include/sound/timer.h b/include/sound/timer.h index 7ae226ab6990..91b9baab50f7 100644 --- a/include/sound/timer.h +++ b/include/sound/timer.h @@ -104,7 +104,7 @@ struct snd_timer_instance { unsigned long ticks, unsigned long resolution); void (*ccallback) (struct snd_timer_instance * timeri, int event, - struct timespec * tstamp, + struct timespec64 * tstamp, unsigned long resolution); void (*disconnect)(struct snd_timer_instance *timeri); void *callback_data; @@ -128,7 +128,7 @@ struct snd_timer_instance { */
int snd_timer_new(struct snd_card *card, char *id, struct snd_timer_id *tid, struct snd_timer **rtimer); -void snd_timer_notify(struct snd_timer *timer, int event, struct timespec *tstamp); +void snd_timer_notify(struct snd_timer *timer, int event, struct timespec64 *tstamp); int snd_timer_global_new(char *id, int device, struct snd_timer **rtimer); int snd_timer_global_free(struct snd_timer *timer); int snd_timer_global_register(struct snd_timer *timer); diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c index b719d0bd833e..e21d3f35a724 100644 --- a/sound/core/pcm_compat.c +++ b/sound/core/pcm_compat.c @@ -229,8 +229,10 @@ static int snd_pcm_status_user_compat(struct snd_pcm_substream *substream, if (clear_user(src, sizeof(*src))) return -EFAULT; if (put_user(status.state, &src->state) || - compat_put_timespec(&status.trigger_tstamp, &src->trigger_tstamp) || - compat_put_timespec(&status.tstamp, &src->tstamp) || + put_user(status.trigger_tstamp.tv_sec, &src->trigger_tstamp.tv_sec) || + put_user(status.trigger_tstamp.tv_sec, &src->trigger_tstamp.tv_nsec) || + put_user(status.tstamp.tv_sec, &src->tstamp.tv_sec) || + put_user(status.tstamp.tv_nsec, &src->tstamp.tv_nsec) || put_user(status.appl_ptr, &src->appl_ptr) || put_user(status.hw_ptr, &src->hw_ptr) || put_user(status.delay, &src->delay) || @@ -239,8 +241,10 @@ static int snd_pcm_status_user_compat(struct snd_pcm_substream *substream, put_user(status.overrange, &src->overrange) || put_user(status.suspended_state, &src->suspended_state) || put_user(status.audio_tstamp_data, &src->audio_tstamp_data) || - compat_put_timespec(&status.audio_tstamp, &src->audio_tstamp) || - compat_put_timespec(&status.driver_tstamp, &src->driver_tstamp) || + put_user(status.audio_tstamp.tv_sec, &src->audio_tstamp.tv_sec) || + put_user(status.audio_tstamp.tv_nsec, &src->audio_tstamp.tv_nsec) || + put_user(status.driver_tstamp.tv_sec, &src->driver_tstamp.tv_sec) || + put_user(status.driver_tstamp.tv_nsec, &src->driver_tstamp.tv_nsec) || put_user(status.audio_tstamp_accuracy, &src->audio_tstamp_accuracy)) return -EFAULT;
@@ -252,8 +256,8 @@ static int snd_pcm_status_user_compat(struct snd_pcm_substream *substream, struct snd_pcm_status_x32 { s32 state; u32 rsvd; /* alignment */ - struct timespec trigger_tstamp; - struct timespec tstamp; + struct __kernel_timespec trigger_tstamp; + struct __kernel_timespec tstamp; u32 appl_ptr; u32 hw_ptr; s32 delay; @@ -262,8 +266,8 @@ struct snd_pcm_status_x32 { u32 overrange; s32 suspended_state; u32 audio_tstamp_data; - struct timespec audio_tstamp; - struct timespec driver_tstamp; + struct __kernel_timespec audio_tstamp; + struct __kernel_timespec driver_tstamp; u32 audio_tstamp_accuracy; unsigned char reserved[52-2*sizeof(struct timespec)]; } __packed; @@ -293,8 +297,10 @@ static int snd_pcm_status_user_x32(struct snd_pcm_substream *substream, if (clear_user(src, sizeof(*src))) return -EFAULT; if (put_user(status.state, &src->state) || - put_timespec(&status.trigger_tstamp, &src->trigger_tstamp) || - put_timespec(&status.tstamp, &src->tstamp) || + put_user(status.trigger_tstamp.tv_sec, &src->trigger_tstamp.tv_sec) || + put_user(status.trigger_tstamp.tv_sec, &src->trigger_tstamp.tv_nsec) || + put_user(status.tstamp.tv_sec, &src->tstamp.tv_sec) || + put_user(status.tstamp.tv_nsec, &src->tstamp.tv_nsec) || put_user(status.appl_ptr, &src->appl_ptr) || put_user(status.hw_ptr, &src->hw_ptr) || put_user(status.delay, &src->delay) || @@ -303,8 +309,10 @@ static int snd_pcm_status_user_x32(struct snd_pcm_substream *substream, put_user(status.overrange, &src->overrange) || put_user(status.suspended_state, &src->suspended_state) || put_user(status.audio_tstamp_data, &src->audio_tstamp_data) || - put_timespec(&status.audio_tstamp, &src->audio_tstamp) || - put_timespec(&status.driver_tstamp, &src->driver_tstamp) || + put_user(status.audio_tstamp.tv_sec, &src->audio_tstamp.tv_sec) || + put_user(status.audio_tstamp.tv_nsec, &src->audio_tstamp.tv_nsec) || + put_user(status.driver_tstamp.tv_sec, &src->driver_tstamp.tv_sec) || + put_user(status.driver_tstamp.tv_nsec, &src->driver_tstamp.tv_nsec) || put_user(status.audio_tstamp_accuracy, &src->audio_tstamp_accuracy)) return -EFAULT;
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index f4a19509cccf..9278b5ded9a2 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -158,8 +158,12 @@ static void xrun(struct snd_pcm_substream *substream) struct snd_pcm_runtime *runtime = substream->runtime;
trace_xrun(substream); - if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) - snd_pcm_gettime(runtime, (struct timespec *)&runtime->status->tstamp); + if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) { + struct timespec64 tstamp; + + snd_pcm_gettime(runtime, &tstamp); + runtime->status->tstamp = timespec64_to_timespec(tstamp); + } snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN); if (xrun_debug(substream, XRUN_DEBUG_BASIC)) { char name[16]; @@ -217,12 +221,12 @@ int snd_pcm_update_state(struct snd_pcm_substream *substream, }
static void update_audio_tstamp(struct snd_pcm_substream *substream, - struct timespec *curr_tstamp, - struct timespec *audio_tstamp) + struct timespec64 *curr_tstamp, + struct timespec64 *audio_tstamp) { struct snd_pcm_runtime *runtime = substream->runtime; u64 audio_frames, audio_nsecs; - struct timespec driver_tstamp; + struct timespec64 driver_tstamp;
if (runtime->tstamp_mode != SNDRV_PCM_TSTAMP_ENABLE) return; @@ -246,18 +250,22 @@ static void update_audio_tstamp(struct snd_pcm_substream *substream, } audio_nsecs = div_u64(audio_frames * 1000000000LL, runtime->rate); - *audio_tstamp = ns_to_timespec(audio_nsecs); + *audio_tstamp = ns_to_timespec64(audio_nsecs); } - if (!timespec_equal(&runtime->status->audio_tstamp, audio_tstamp)) { - runtime->status->audio_tstamp = *audio_tstamp; - runtime->status->tstamp = *curr_tstamp; + + if (runtime->status->audio_tstamp.tv_sec != audio_tstamp->tv_sec || + runtime->status->audio_tstamp.tv_nsec != audio_tstamp->tv_nsec) { + runtime->status->audio_tstamp = + timespec64_to_timespec(*audio_tstamp); + runtime->status->tstamp = timespec64_to_timespec(*curr_tstamp); }
+ /* * re-take a driver timestamp to let apps detect if the reference tstamp * read by low-level hardware was provided with a delay */ - snd_pcm_gettime(substream->runtime, (struct timespec *)&driver_tstamp); + snd_pcm_gettime(substream->runtime, &driver_tstamp); runtime->driver_tstamp = driver_tstamp; }
@@ -270,8 +278,8 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream, snd_pcm_sframes_t hdelta, delta; unsigned long jdelta; unsigned long curr_jiffies; - struct timespec curr_tstamp; - struct timespec audio_tstamp; + struct timespec64 curr_tstamp; + struct timespec64 audio_tstamp; int crossed_boundary = 0;
old_hw_ptr = runtime->status->hw_ptr; @@ -294,9 +302,9 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
/* re-test in case tstamp type is not supported in hardware and was demoted to DEFAULT */ if (runtime->audio_tstamp_report.actual_type == SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT) - snd_pcm_gettime(runtime, (struct timespec *)&curr_tstamp); + snd_pcm_gettime(runtime, &curr_tstamp); } else - snd_pcm_gettime(runtime, (struct timespec *)&curr_tstamp); + snd_pcm_gettime(runtime, &curr_tstamp); }
if (pos == SNDRV_PCM_POS_XRUN) { diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 35ffccea94c3..c57dd4b30198 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -884,12 +884,12 @@ int snd_pcm_status(struct snd_pcm_substream *substream, status->suspended_state = runtime->status->suspended_state; if (status->state == SNDRV_PCM_STATE_OPEN) goto _end; - status->trigger_tstamp = runtime->trigger_tstamp; + status->trigger_tstamp = timespec64_to_timespec(runtime->trigger_tstamp); if (snd_pcm_running(substream)) { snd_pcm_update_hw_ptr(substream); if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) { status->tstamp = runtime->status->tstamp; - status->driver_tstamp = runtime->driver_tstamp; + status->driver_tstamp = timespec64_to_timespec(runtime->driver_tstamp); status->audio_tstamp = runtime->status->audio_tstamp; if (runtime->audio_tstamp_report.valid == 1) @@ -902,8 +902,12 @@ int snd_pcm_status(struct snd_pcm_substream *substream, } } else { /* get tstamp only in fallback mode and only if enabled */ - if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) - snd_pcm_gettime(runtime, &status->tstamp); + if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) { + struct timespec64 tstamp; + + snd_pcm_gettime(runtime, &tstamp); + status->tstamp = timespec64_to_timespec(tstamp); + } } _tstamp_end: status->appl_ptr = runtime->control->appl_ptr; diff --git a/sound/core/timer.c b/sound/core/timer.c index dc87728c5b74..a77b4619f1b7 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -73,7 +73,7 @@ struct snd_timer_user { spinlock_t qlock; unsigned long last_resolution; unsigned int filter; - struct timespec tstamp; /* trigger tstamp */ + struct timespec64 tstamp; /* trigger tstamp */ wait_queue_head_t qchange_sleep; struct fasync_struct *fasync; struct mutex ioctl_lock; @@ -448,12 +448,12 @@ static void snd_timer_notify1(struct snd_timer_instance *ti, int event) struct snd_timer *timer; unsigned long resolution = 0; struct snd_timer_instance *ts; - struct timespec tstamp; + struct timespec64 tstamp;
if (timer_tstamp_monotonic) - ktime_get_ts(&tstamp); + ktime_get_ts64(&tstamp); else - getnstimeofday(&tstamp); + ktime_get_real_ts64(&tstamp); if (snd_BUG_ON(event < SNDRV_TIMER_EVENT_START || event > SNDRV_TIMER_EVENT_PAUSE)) return; @@ -998,7 +998,7 @@ static int snd_timer_dev_disconnect(struct snd_device *device) return 0; }
-void snd_timer_notify(struct snd_timer *timer, int event, struct timespec *tstamp) +void snd_timer_notify(struct snd_timer *timer, int event, struct timespec64 *tstamp) { unsigned long flags; unsigned long resolution = 0; @@ -1295,7 +1295,7 @@ static void snd_timer_user_append_to_tqueue(struct snd_timer_user *tu,
static void snd_timer_user_ccallback(struct snd_timer_instance *timeri, int event, - struct timespec *tstamp, + struct timespec64 *tstamp, unsigned long resolution) { struct snd_timer_user *tu = timeri->callback_data; @@ -1309,7 +1309,7 @@ static void snd_timer_user_ccallback(struct snd_timer_instance *timeri, return; memset(&r1, 0, sizeof(r1)); r1.event = event; - r1.tstamp = *tstamp; + r1.tstamp = timespec64_to_timespec(*tstamp); r1.val = resolution; spin_lock_irqsave(&tu->qlock, flags); snd_timer_user_append_to_tqueue(tu, &r1); @@ -1332,7 +1332,7 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri, { struct snd_timer_user *tu = timeri->callback_data; struct snd_timer_tread *r, r1; - struct timespec tstamp; + struct timespec64 tstamp; int prev, append = 0;
memset(&r1, 0, sizeof(r1)); @@ -1345,14 +1345,14 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri, } if (tu->last_resolution != resolution || ticks > 0) { if (timer_tstamp_monotonic) - ktime_get_ts(&tstamp); + ktime_get_ts64(&tstamp); else - getnstimeofday(&tstamp); + ktime_get_real_ts64(&tstamp); } if ((tu->filter & (1 << SNDRV_TIMER_EVENT_RESOLUTION)) && tu->last_resolution != resolution) { r1.event = SNDRV_TIMER_EVENT_RESOLUTION; - r1.tstamp = tstamp; + r1.tstamp = timespec64_to_timespec(tstamp); r1.val = resolution; snd_timer_user_append_to_tqueue(tu, &r1); tu->last_resolution = resolution; @@ -1366,14 +1366,14 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri, prev = tu->qtail == 0 ? tu->queue_size - 1 : tu->qtail - 1; r = &tu->tqueue[prev]; if (r->event == SNDRV_TIMER_EVENT_TICK) { - r->tstamp = tstamp; + r->tstamp = timespec64_to_timespec(tstamp); r->val += ticks; append++; goto __wake; } } r1.event = SNDRV_TIMER_EVENT_TICK; - r1.tstamp = tstamp; + r1.tstamp = timespec64_to_timespec(tstamp); r1.val = ticks; snd_timer_user_append_to_tqueue(tu, &r1); append++; @@ -1852,7 +1852,7 @@ static int snd_timer_user_status(struct file *file, if (!tu->timeri) return -EBADFD; memset(&status, 0, sizeof(status)); - status.tstamp = tu->tstamp; + status.tstamp = timespec64_to_timespec(tu->tstamp); status.resolution = snd_timer_resolution(tu->timeri); status.lost = tu->timeri->lost; status.overrun = tu->overrun; diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c index d1eb14842340..c3e45168b848 100644 --- a/sound/pci/hda/hda_controller.c +++ b/sound/pci/hda/hda_controller.c @@ -502,7 +502,7 @@ static inline bool is_link_time_supported(struct snd_pcm_runtime *runtime, }
static int azx_get_time_info(struct snd_pcm_substream *substream, - struct timespec *system_ts, struct timespec *audio_ts, + struct timespec64 *system_ts, struct timespec64 *audio_ts, struct snd_pcm_audio_tstamp_config *audio_tstamp_config, struct snd_pcm_audio_tstamp_report *audio_tstamp_report) { @@ -522,7 +522,7 @@ static int azx_get_time_info(struct snd_pcm_substream *substream, if (audio_tstamp_config->report_delay) nsec = azx_adjust_codec_delay(substream, nsec);
- *audio_ts = ns_to_timespec(nsec); + *audio_ts = ns_to_timespec64(nsec);
audio_tstamp_report->actual_type = SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK; audio_tstamp_report->accuracy_report = 1; /* rest of structure is valid */ @@ -539,16 +539,16 @@ static int azx_get_time_info(struct snd_pcm_substream *substream, return -EINVAL;
case SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW: - *system_ts = ktime_to_timespec(xtstamp.sys_monoraw); + *system_ts = ktime_to_timespec64(xtstamp.sys_monoraw); break;
default: - *system_ts = ktime_to_timespec(xtstamp.sys_realtime); + *system_ts = ktime_to_timespec64(xtstamp.sys_realtime); break;
}
- *audio_ts = ktime_to_timespec(xtstamp.device); + *audio_ts = ktime_to_timespec64(xtstamp.device);
audio_tstamp_report->actual_type = SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK_SYNCHRONIZED; diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index 15cb8ac3e374..a69b56fc3417 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -1163,7 +1163,7 @@ static u64 skl_adjust_codec_delay(struct snd_pcm_substream *substream, }
static int skl_get_time_info(struct snd_pcm_substream *substream, - struct timespec *system_ts, struct timespec *audio_ts, + struct timespec64 *system_ts, struct timespec64 *audio_ts, struct snd_pcm_audio_tstamp_config *audio_tstamp_config, struct snd_pcm_audio_tstamp_report *audio_tstamp_report) { @@ -1181,7 +1181,7 @@ static int skl_get_time_info(struct snd_pcm_substream *substream, if (audio_tstamp_config->report_delay) nsec = skl_adjust_codec_delay(substream, nsec);
- *audio_ts = ns_to_timespec(nsec); + *audio_ts = ns_to_timespec64(nsec);
audio_tstamp_report->actual_type = SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK; audio_tstamp_report->accuracy_report = 1; /* rest of struct is valid */
From: Baolin Wang baolin.wang@linaro.org
The struct snd_ctl_elem_value will use 'timespec' type variables to record timestamp, which is not year 2038 safe on 32bits system.
Since there are no drivers will implemented the tstamp member of the struct snd_ctl_elem_value, and also the stucture size will not be changed if we change timespec to s64 for tstamp member of struct snd_ctl_elem_value.
From Takashi's comments, "In the library, applications are not expected
to access to this structure directly. The applications get opaque pointer to the structure and must use any control APIs to operate it. Actually the library produce no API to handle 'struct snd_ctl_elem_value.tstamp'. This means that we can drop this member from alsa-lib without decline of functionality." Thus we can simply remove the tstamp member to avoid using the type which is not year 2038 safe on 32bits system.
Signed-off-by: Baolin Wang baolin.wang@linaro.org Signed-off-by: Arnd Bergmann arnd@arndb.de --- include/uapi/sound/asound.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index ed0a120d4f08..1231f0a943f1 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -954,8 +954,7 @@ struct snd_ctl_elem_value { } bytes; struct snd_aes_iec958 iec958; } value; /* RO */ - struct timespec tstamp; - unsigned char reserved[128-sizeof(struct timespec)]; + unsigned char reserved[128]; };
struct snd_ctl_tlv {
This changes the user API for ALSA to not rely on time_t as the type any more, so it can survive the update to new C libraries that redefine time_t to 64 bit.
This is a much simpler approach than earlier patches, simply defining the API to use '__kernel_ulong_t' for tv_sec and tv_nsec, keeping the existing binary interface but changing the way we express it.
The downside of this approach is that it requires significant user space changes in alsa-lib and simialr projects in order to convert back the 32-bit timestamps into 'timespec' for consumption by applications, and that it requires using monotonic timestamps to avoid overflowing a timespec in y2038.
To try to counter the incompatibility with existing user sources, the new type is only used when __USE_TIME_BITS64 is set. This gets defined by glibc when an application is compiled with 64-bit time_t.
Unfortunately, existing alsa-lib releases come with their own copy of sound/asound.h and don't use the version provided by the linux/libc headers, so alsa-lib is still silently broken when rebuilt with a new libc until it gets updated to use the new header.
I also try to be extra conservative with the naming of the structure, giving it the rather long name 'snd_monotonic_timestamp' to clarify that this can only be used for monotonic timestamps after we have decided not to extend the current interface to 64 bit stamps. A separate patch is used to allow enforcing the use of monotonic timestamps in the kernel.
Signed-off-by: Arnd Bergmann arnd@arndb.de --- include/sound/asound.h | 8 ++++++++ include/uapi/sound/asound.h | 46 +++++++++++++++++++++++++++++++++++---------- sound/core/compat.h | 11 +++++++++++ sound/core/pcm_compat.c | 38 +++++++++++++++++++------------------ sound/core/pcm_lib.c | 6 +++--- sound/core/pcm_native.c | 9 ++++----- sound/core/rawmidi_compat.c | 12 ++++++------ sound/core/timer.c | 10 +++++----- sound/core/timer_compat.c | 4 +++- 9 files changed, 96 insertions(+), 48 deletions(-) create mode 100644 sound/core/compat.h
diff --git a/include/sound/asound.h b/include/sound/asound.h index c2dff5369d33..8c919eb9bb45 100644 --- a/include/sound/asound.h +++ b/include/sound/asound.h @@ -37,4 +37,12 @@ #endif
#include <uapi/sound/asound.h> + + +static inline struct snd_monotonic_timestamp +timespec64_to_snd_monotonic_timestamp(struct timespec64 ts) +{ + return (struct snd_monotonic_timestamp) { (u32)ts.tv_sec, ts.tv_nsec }; +}; + #endif /* __SOUND_ASOUND_H */ diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index 1231f0a943f1..9c61cf77beb8 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -455,10 +455,36 @@ enum { SNDRV_PCM_AUDIO_TSTAMP_TYPE_LAST = SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK_SYNCHRONIZED };
+#if (__BITS_PER_LONG == 32 && defined(__USE_TIME_BITS64)) || defined __KERNEL__ +/* + * We used to use 'struct timespec' here, but that no longer works on + * 32 bit architectures since the migration to 64-bit time_t in user + * space. Rather than updating all ioctls, we change the internal type + * to a new one with the same binary layout as before. + * + * We use a 'unsigned long' as the base type here to give us a little + * extra range over the traditional signed type, but this really + * should only be used for CLOCK_MONOTONIC timestamps, not CLOCK_REALTIME, + * to avoid all issues with y2038 overflow, hence the name. + * + * alsa-lib 1.1.6 and earlier are incompatible with this definition and + * will break either at compile time or at runtime if built against + * an older header while using a 64-bit time_t on a 32-bit architecture, + * so if you run into a build problem here, please upgrade to the latest + * alsa-lib. + */ +struct snd_monotonic_timestamp { + __kernel_ulong_t tv_sec; + __kernel_ulong_t tv_nsec; +}; +#else +#define snd_monotonic_timestamp timespec +#endif + struct snd_pcm_status { snd_pcm_state_t state; /* stream state */ - struct timespec trigger_tstamp; /* time when stream was started/stopped/paused */ - struct timespec tstamp; /* reference timestamp */ + struct snd_monotonic_timestamp trigger_tstamp;/* time when stream was started/stopped/paused */ + struct snd_monotonic_timestamp tstamp; /* reference timestamp */ snd_pcm_uframes_t appl_ptr; /* appl ptr */ snd_pcm_uframes_t hw_ptr; /* hw ptr */ snd_pcm_sframes_t delay; /* current delay in frames */ @@ -467,19 +493,19 @@ struct snd_pcm_status { snd_pcm_uframes_t overrange; /* count of ADC (capture) overrange detections from last status */ snd_pcm_state_t suspended_state; /* suspended stream state */ __u32 audio_tstamp_data; /* needed for 64-bit alignment, used for configs/report to/from userspace */ - struct timespec audio_tstamp; /* sample counter, wall clock, PHC or on-demand sync'ed */ - struct timespec driver_tstamp; /* useful in case reference system tstamp is reported with delay */ + struct snd_monotonic_timestamp audio_tstamp; /* sample counter, wall clock, PHC or on-demand sync'ed */ + struct snd_monotonic_timestamp driver_tstamp; /* useful in case reference system tstamp is reported with delay */ __u32 audio_tstamp_accuracy; /* in ns units, only valid if indicated in audio_tstamp_data */ - unsigned char reserved[52-2*sizeof(struct timespec)]; /* must be filled with zero */ + unsigned char reserved[52-2*sizeof(struct snd_monotonic_timestamp)]; /* must be filled with zero */ };
struct snd_pcm_mmap_status { snd_pcm_state_t state; /* RO: state - SNDRV_PCM_STATE_XXXX */ int pad1; /* Needed for 64 bit alignment */ snd_pcm_uframes_t hw_ptr; /* RO: hw ptr (0...boundary-1) */ - struct timespec tstamp; /* Timestamp */ + struct snd_monotonic_timestamp tstamp; /* Timestamp */ snd_pcm_state_t suspended_state; /* RO: suspended stream state */ - struct timespec audio_tstamp; /* from sample counter or wall clock */ + struct snd_monotonic_timestamp audio_tstamp; /* from sample counter or wall clock */ };
struct snd_pcm_mmap_control { @@ -649,7 +675,7 @@ struct snd_rawmidi_params {
struct snd_rawmidi_status { int stream; - struct timespec tstamp; /* Timestamp */ + struct snd_monotonic_timestamp tstamp; /* Timestamp */ size_t avail; /* available bytes */ size_t xruns; /* count of overruns since last status (in bytes) */ unsigned char reserved[16]; /* reserved for future use */ @@ -761,7 +787,7 @@ struct snd_timer_params { };
struct snd_timer_status { - struct timespec tstamp; /* Timestamp - last update */ + struct snd_monotonic_timestamp tstamp; /* Timestamp - last update */ unsigned int resolution; /* current period resolution in ns */ unsigned int lost; /* counter of master tick lost */ unsigned int overrun; /* count of read queue overruns */ @@ -811,7 +837,7 @@ enum {
struct snd_timer_tread { int event; - struct timespec tstamp; + struct snd_monotonic_timestamp tstamp; unsigned int val; };
diff --git a/sound/core/compat.h b/sound/core/compat.h new file mode 100644 index 000000000000..7a08d6e34955 --- /dev/null +++ b/sound/core/compat.h @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0 + +#ifndef SND_CORE_COMPAT_H +#define SND_CORE_COMPAT_H + +struct compat_snd_monotonic_timestamp { + __u32 tv_sec; + __u32 tv_nsec; +}; + +#endif diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c index e21d3f35a724..40e9be542322 100644 --- a/sound/core/pcm_compat.c +++ b/sound/core/pcm_compat.c @@ -23,6 +23,8 @@ #include <linux/compat.h> #include <linux/slab.h>
+#include "compat.h" + static int snd_pcm_ioctl_delay_compat(struct snd_pcm_substream *substream, s32 __user *src) { @@ -189,8 +191,8 @@ static int snd_pcm_channel_info_user(struct snd_pcm_substream *substream,
struct snd_pcm_status32 { s32 state; - struct compat_timespec trigger_tstamp; - struct compat_timespec tstamp; + struct compat_snd_monotonic_timestamp trigger_tstamp; + struct compat_snd_monotonic_timestamp tstamp; u32 appl_ptr; u32 hw_ptr; s32 delay; @@ -199,10 +201,10 @@ struct snd_pcm_status32 { u32 overrange; s32 suspended_state; u32 audio_tstamp_data; - struct compat_timespec audio_tstamp; - struct compat_timespec driver_tstamp; + struct compat_snd_monotonic_timestamp audio_tstamp; + struct compat_snd_monotonic_timestamp driver_tstamp; u32 audio_tstamp_accuracy; - unsigned char reserved[52-2*sizeof(struct compat_timespec)]; + unsigned char reserved[52-2*sizeof(struct compat_snd_monotonic_timestamp)]; } __attribute__((packed));
@@ -269,11 +271,9 @@ struct snd_pcm_status_x32 { struct __kernel_timespec audio_tstamp; struct __kernel_timespec driver_tstamp; u32 audio_tstamp_accuracy; - unsigned char reserved[52-2*sizeof(struct timespec)]; + unsigned char reserved[52-2*sizeof(struct __kernel_timespec)]; } __packed;
-#define put_timespec(src, dst) copy_to_user(dst, src, sizeof(*dst)) - static int snd_pcm_status_user_x32(struct snd_pcm_substream *substream, struct snd_pcm_status_x32 __user *src, bool ext) @@ -461,14 +461,13 @@ static int snd_pcm_ioctl_xfern_compat(struct snd_pcm_substream *substream, return err; }
- struct snd_pcm_mmap_status32 { s32 state; s32 pad1; u32 hw_ptr; - struct compat_timespec tstamp; + struct compat_snd_monotonic_timestamp tstamp; s32 suspended_state; - struct compat_timespec audio_tstamp; + struct compat_snd_monotonic_timestamp audio_tstamp; } __attribute__((packed));
struct snd_pcm_mmap_control32 { @@ -535,10 +534,11 @@ static int snd_pcm_ioctl_sync_ptr_compat(struct snd_pcm_substream *substream, snd_pcm_stream_unlock_irq(substream); if (put_user(sstatus.state, &src->s.status.state) || put_user(sstatus.hw_ptr, &src->s.status.hw_ptr) || - compat_put_timespec(&sstatus.tstamp, &src->s.status.tstamp) || + put_user(sstatus.tstamp.tv_sec, &src->s.status.tstamp.tv_sec) || + put_user(sstatus.tstamp.tv_nsec, &src->s.status.tstamp.tv_nsec) || put_user(sstatus.suspended_state, &src->s.status.suspended_state) || - compat_put_timespec(&sstatus.audio_tstamp, - &src->s.status.audio_tstamp) || + put_user(sstatus.audio_tstamp.tv_sec, &src->s.status.audio_tstamp.tv_sec) || + put_user(sstatus.audio_tstamp.tv_nsec, &src->s.status.audio_tstamp.tv_nsec) || put_user(scontrol.appl_ptr, &src->c.control.appl_ptr) || put_user(scontrol.avail_min, &src->c.control.avail_min)) return -EFAULT; @@ -553,10 +553,10 @@ struct snd_pcm_mmap_status_x32 { s32 pad1; u32 hw_ptr; u32 pad2; /* alignment */ - struct timespec tstamp; + struct __kernel_timespec tstamp; s32 suspended_state; s32 pad3; - struct timespec audio_tstamp; + struct __kernel_timespec audio_tstamp; } __packed;
struct snd_pcm_mmap_control_x32 { @@ -624,9 +624,11 @@ static int snd_pcm_ioctl_sync_ptr_x32(struct snd_pcm_substream *substream, snd_pcm_stream_unlock_irq(substream); if (put_user(sstatus.state, &src->s.status.state) || put_user(sstatus.hw_ptr, &src->s.status.hw_ptr) || - put_timespec(&sstatus.tstamp, &src->s.status.tstamp) || + put_user(sstatus.tstamp.tv_sec, &src->s.status.tstamp.tv_sec) || + put_user(sstatus.tstamp.tv_nsec, &src->s.status.tstamp.tv_nsec) || put_user(sstatus.suspended_state, &src->s.status.suspended_state) || - put_timespec(&sstatus.audio_tstamp, &src->s.status.audio_tstamp) || + put_user(sstatus.audio_tstamp.tv_sec, &src->s.status.audio_tstamp.tv_sec) || + put_user(sstatus.audio_tstamp.tv_nsec, &src->s.status.audio_tstamp.tv_nsec) || put_user(scontrol.appl_ptr, &src->c.control.appl_ptr) || put_user(scontrol.avail_min, &src->c.control.avail_min)) return -EFAULT; diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 9278b5ded9a2..e676356bd3be 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -162,7 +162,7 @@ static void xrun(struct snd_pcm_substream *substream) struct timespec64 tstamp;
snd_pcm_gettime(runtime, &tstamp); - runtime->status->tstamp = timespec64_to_timespec(tstamp); + runtime->status->tstamp = timespec64_to_snd_monotonic_timestamp(tstamp); } snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN); if (xrun_debug(substream, XRUN_DEBUG_BASIC)) { @@ -256,8 +256,8 @@ static void update_audio_tstamp(struct snd_pcm_substream *substream, if (runtime->status->audio_tstamp.tv_sec != audio_tstamp->tv_sec || runtime->status->audio_tstamp.tv_nsec != audio_tstamp->tv_nsec) { runtime->status->audio_tstamp = - timespec64_to_timespec(*audio_tstamp); - runtime->status->tstamp = timespec64_to_timespec(*curr_tstamp); + timespec64_to_snd_monotonic_timestamp(*audio_tstamp); + runtime->status->tstamp = timespec64_to_snd_monotonic_timestamp(*curr_tstamp); }
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index c57dd4b30198..d27c6252e14c 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -884,14 +884,13 @@ int snd_pcm_status(struct snd_pcm_substream *substream, status->suspended_state = runtime->status->suspended_state; if (status->state == SNDRV_PCM_STATE_OPEN) goto _end; - status->trigger_tstamp = timespec64_to_timespec(runtime->trigger_tstamp); + status->trigger_tstamp = timespec64_to_snd_monotonic_timestamp(runtime->trigger_tstamp); if (snd_pcm_running(substream)) { snd_pcm_update_hw_ptr(substream); if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) { status->tstamp = runtime->status->tstamp; - status->driver_tstamp = timespec64_to_timespec(runtime->driver_tstamp); - status->audio_tstamp = - runtime->status->audio_tstamp; + status->driver_tstamp = timespec64_to_snd_monotonic_timestamp(runtime->driver_tstamp); + status->audio_tstamp = runtime->status->audio_tstamp; if (runtime->audio_tstamp_report.valid == 1) /* backwards compatibility, no report provided in COMPAT mode */ snd_pcm_pack_audio_tstamp_report(&status->audio_tstamp_data, @@ -906,7 +905,7 @@ int snd_pcm_status(struct snd_pcm_substream *substream, struct timespec64 tstamp;
snd_pcm_gettime(runtime, &tstamp); - status->tstamp = timespec64_to_timespec(tstamp); + status->tstamp = timespec64_to_snd_monotonic_timestamp(tstamp); } } _tstamp_end: diff --git a/sound/core/rawmidi_compat.c b/sound/core/rawmidi_compat.c index f69764d7cdd7..bf8b0c9da690 100644 --- a/sound/core/rawmidi_compat.c +++ b/sound/core/rawmidi_compat.c @@ -55,7 +55,7 @@ static int snd_rawmidi_ioctl_params_compat(struct snd_rawmidi_file *rfile,
struct snd_rawmidi_status32 { s32 stream; - struct compat_timespec tstamp; + struct snd_monotonic_timestamp tstamp; u32 avail; u32 xruns; unsigned char reserved[16]; @@ -85,7 +85,8 @@ static int snd_rawmidi_ioctl_status_compat(struct snd_rawmidi_file *rfile, if (err < 0) return err;
- if (compat_put_timespec(&status.tstamp, &src->tstamp) || + if (put_user(status.tstamp.tv_sec, &src->tstamp.tv_sec) || + put_user(status.tstamp.tv_nsec, &src->tstamp.tv_nsec) || put_user(status.avail, &src->avail) || put_user(status.xruns, &src->xruns)) return -EFAULT; @@ -98,14 +99,12 @@ static int snd_rawmidi_ioctl_status_compat(struct snd_rawmidi_file *rfile, struct snd_rawmidi_status_x32 { s32 stream; u32 rsvd; /* alignment */ - struct timespec tstamp; + struct __kernel_timespec tstamp; u32 avail; u32 xruns; unsigned char reserved[16]; } __attribute__((packed));
-#define put_timespec(src, dst) copy_to_user(dst, src, sizeof(*dst)) - static int snd_rawmidi_ioctl_status_x32(struct snd_rawmidi_file *rfile, struct snd_rawmidi_status_x32 __user *src) { @@ -130,7 +129,8 @@ static int snd_rawmidi_ioctl_status_x32(struct snd_rawmidi_file *rfile, if (err < 0) return err;
- if (put_timespec(&status.tstamp, &src->tstamp) || + if (put_user(status.tstamp.tv_sec, &src->tstamp.tv_sec) || + put_user(status.tstamp.tv_nsec, &src->tstamp.tv_nsec) || put_user(status.avail, &src->avail) || put_user(status.xruns, &src->xruns)) return -EFAULT; diff --git a/sound/core/timer.c b/sound/core/timer.c index a77b4619f1b7..b7059a9e9edc 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -1309,7 +1309,7 @@ static void snd_timer_user_ccallback(struct snd_timer_instance *timeri, return; memset(&r1, 0, sizeof(r1)); r1.event = event; - r1.tstamp = timespec64_to_timespec(*tstamp); + r1.tstamp = timespec64_to_snd_monotonic_timestamp(*tstamp); r1.val = resolution; spin_lock_irqsave(&tu->qlock, flags); snd_timer_user_append_to_tqueue(tu, &r1); @@ -1352,7 +1352,7 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri, if ((tu->filter & (1 << SNDRV_TIMER_EVENT_RESOLUTION)) && tu->last_resolution != resolution) { r1.event = SNDRV_TIMER_EVENT_RESOLUTION; - r1.tstamp = timespec64_to_timespec(tstamp); + r1.tstamp = timespec64_to_snd_monotonic_timestamp(tstamp); r1.val = resolution; snd_timer_user_append_to_tqueue(tu, &r1); tu->last_resolution = resolution; @@ -1366,14 +1366,14 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri, prev = tu->qtail == 0 ? tu->queue_size - 1 : tu->qtail - 1; r = &tu->tqueue[prev]; if (r->event == SNDRV_TIMER_EVENT_TICK) { - r->tstamp = timespec64_to_timespec(tstamp); + r->tstamp = timespec64_to_snd_monotonic_timestamp(tstamp); r->val += ticks; append++; goto __wake; } } r1.event = SNDRV_TIMER_EVENT_TICK; - r1.tstamp = timespec64_to_timespec(tstamp); + r1.tstamp = timespec64_to_snd_monotonic_timestamp(tstamp); r1.val = ticks; snd_timer_user_append_to_tqueue(tu, &r1); append++; @@ -1852,7 +1852,7 @@ static int snd_timer_user_status(struct file *file, if (!tu->timeri) return -EBADFD; memset(&status, 0, sizeof(status)); - status.tstamp = timespec64_to_timespec(tu->tstamp); + status.tstamp = timespec64_to_snd_monotonic_timestamp(tu->tstamp); status.resolution = snd_timer_resolution(tu->timeri); status.lost = tu->timeri->lost; status.overrun = tu->overrun; diff --git a/sound/core/timer_compat.c b/sound/core/timer_compat.c index e00f7e399e46..960d1075071f 100644 --- a/sound/core/timer_compat.c +++ b/sound/core/timer_compat.c @@ -22,6 +22,8 @@
#include <linux/compat.h>
+#include "compat.h" + /* * ILP32/LP64 has different size for 'long' type. Additionally, the size * of storage alignment differs depending on architectures. Here, '__packed' @@ -84,7 +86,7 @@ static int snd_timer_user_info_compat(struct file *file, }
struct snd_timer_status32 { - struct compat_timespec tstamp; + struct compat_snd_monotonic_timestamp tstamp; u32 resolution; u32 lost; u32 overrun;
The kernel has supported monotonic timestamps since linux-2.6.25, but it still defaults to CLOCK_REALTIME, which has multiple problems: It skips backwards for every leap second, it may skip at any time from settimeofday() or NTP updates, and it overflows in year 2038.
alsa-lib already tries to use monotonic timestamps whenever they are available, so user space should generally be fine with it.
To be on the safe side, I'm adding a Kconfig option that allows retaining the current behavior, while making the default to disallow CLOCK_REALTIME stamps. That default makes sense because of the decision to not extend the ioctl interface to 64-bit time stamps. We need today's kernels to run beyond 2038 in certain embedded markets, so it's better to discover any possible problems now rather than running into them only after time stamps overflow.
I'm defaulting to SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW behavior rather than SNDRV_PCM_TSTAMP_TYPE_MONOTONIC since the latter is not supported by the HDA hardware timestamps.
A similar change was done in the DRM subsystem and it turned out that nothing relied on CLOCK_REALTIME behavior for their timestamps, and the option was subsequently removed. If the same turns out to be true here, we can do the same and clean it out in a few years.
Signed-off-by: Arnd Bergmann arnd@arndb.de --- include/sound/pcm.h | 4 ++++ include/uapi/sound/asound.h | 4 +++- sound/core/Kconfig | 11 +++++++++++ sound/core/pcm.c | 3 +++ sound/core/pcm_native.c | 10 ++++++++++ sound/pci/hda/hda_controller.c | 4 ++++ 6 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 973d6745217a..5d5daa190b08 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -1190,7 +1190,11 @@ static inline void snd_pcm_gettime(struct snd_pcm_runtime *runtime, getrawmonotonic64(tv); break; default: +#ifdef CONFIG_SND_TSTAMP_REALTIME ktime_get_real_ts64(tv); +#else + ktime_get_ts64(tv); +#endif break; } } diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index 9c61cf77beb8..1d8546731306 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -542,8 +542,10 @@ struct snd_xfern { };
enum { +#if !(__BITS_PER_LONG == 32 && defined(__USE_TIME_BITS64)) SNDRV_PCM_TSTAMP_TYPE_GETTIMEOFDAY = 0, /* gettimeofday equivalent */ - SNDRV_PCM_TSTAMP_TYPE_MONOTONIC, /* posix_clock_monotonic equivalent */ +#endif + SNDRV_PCM_TSTAMP_TYPE_MONOTONIC = 1, /* posix_clock_monotonic equivalent */ SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW, /* monotonic_raw (no NTP) */ SNDRV_PCM_TSTAMP_TYPE_LAST = SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW, }; diff --git a/sound/core/Kconfig b/sound/core/Kconfig index 6e937a8146a1..6dedb0661fa4 100644 --- a/sound/core/Kconfig +++ b/sound/core/Kconfig @@ -100,6 +100,17 @@ config SND_HRTIMER To compile this driver as a module, choose M here: the module will be called snd-hrtimer.
+config SND_TSTAMP_REALTIME + bool "allow CLOCK_REALTIME timestamps" + help + Say Y here if you have user space applications that rely on + the traditional CLOCK_REALTIME based timestamps rather than + using CLOCK_MONOTONIC or CLOCK_MONOTONIC_RAW. + CLOCK_REALTIME has problems with overflow in year 2038, with + leap seconds and concurrent time updates (e.g. through NTP). + + If unsure, say N. + config SND_DYNAMIC_MINORS bool "Dynamic device file minor numbers" help diff --git a/sound/core/pcm.c b/sound/core/pcm.c index 66ac89aad681..aaed96566bde 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -1028,6 +1028,9 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
init_waitqueue_head(&runtime->sleep); init_waitqueue_head(&runtime->tsleep); +#ifdef CONFIG_SND_TSTAMP_REALTIME + runtime->tstamp_type = SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW; +#endif
runtime->status->state = SNDRV_PCM_STATE_OPEN;
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index d27c6252e14c..a2196208b057 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -811,6 +811,11 @@ static int snd_pcm_sw_params(struct snd_pcm_substream *substream, if (params->proto >= SNDRV_PROTOCOL_VERSION(2, 0, 12) && params->tstamp_type > SNDRV_PCM_TSTAMP_TYPE_LAST) return -EINVAL; +#ifndef CONFIG_SND_TSTAMP_REALTIME + /* all other types are invalid here, and fall back to raw mono */ + if (params->tstamp_type != SNDRV_PCM_TSTAMP_TYPE_MONOTONIC) + params->tstamp_type = SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW; +#endif if (params->avail_min == 0) return -EINVAL; if (params->silence_size >= runtime->boundary) { @@ -2769,6 +2774,11 @@ static int snd_pcm_tstamp(struct snd_pcm_substream *substream, int __user *_arg) return -EFAULT; if (arg < 0 || arg > SNDRV_PCM_TSTAMP_TYPE_LAST) return -EINVAL; +#ifndef CONFIG_SND_TSTAMP_REALTIME + /* all other types are invalid here, and fall back to raw mono */ + if (arg != SNDRV_PCM_TSTAMP_TYPE_MONOTONIC) + arg = SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW; +#endif runtime->tstamp_type = arg; return 0; } diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c index c3e45168b848..a2fca77582d5 100644 --- a/sound/pci/hda/hda_controller.c +++ b/sound/pci/hda/hda_controller.c @@ -543,7 +543,11 @@ static int azx_get_time_info(struct snd_pcm_substream *substream, break;
default: +#ifdef CONFIG_SND_TSTAMP_REALTIME *system_ts = ktime_to_timespec64(xtstamp.sys_realtime); +#else + *system_ts = ktime_to_timespec64(xtstamp.sys_monoraw); +#endif break;
}
Dne 26.4.2018 v 14:44 Arnd Bergmann napsal(a):
I've tried the suggestion from Jaroslaw, doing a minimal change to the UAPI headers to keep the existing binary interface. As he predicted, this is a much simpler set of kernel changes, but we will pay for that with added complexity in alsa-lib.
The first two patches in this series are taken from Baolin's patch set, with a small bugfix folded in to avoid a compile-time regression.
The other two patches are to redefine the UAPI and to deprecate the support for CLOCK_REALTIME time stamps, which we can no longer allow with user space that we expect to survive beyond 2038.
Overall, I'd still be happier with Baolin's approach since it allows us to keep compatiblity with CLOCK_REALTIME users and requires fewer changes in user space, but this would work as well.
Hi Arnd,
Thanks for your work. I proposed a bit different implementation. Example:
struct snd_example { struct snd_native_timespec tstamp; .... u64 tstamp_sec64; /* use the reserved[] array for this */ };
So tstamp contains the current 32-bit tv_sec/tv_nsec and the full 64-bit value is in tstamp_sec64. In this way, we can transfer any type of the timespec64 values and it's backward compatible to retain the binary compatibility. The protocol versions should be increased to let the userspace know about the new 64-bit fields.
The timer read protocol must be updated, because the stream will change, so I am fine to add new ioctl (like originally proposed).
The alsa-lib defines timespec only if posix defines are not set so glibc's time.h does not define the timespec structure - it may be improved.
Jaroslav
Arnd
Cc: Jaroslav Kysela perex@perex.cz Cc: tiwai@suse.com Cc: lgirdwood@gmail.com Cc: broonie@kernel.org Cc: o-takashi@sakamocchi.jp Cc: y2038@lists.linaro.org Cc: alsa-devel@alsa-project.org Cc: linux-kernel@vger.kernel.org Cc: Baolin Wang baolin.wang@linaro.org
Arnd Bergmann (2): ALSA: replace timespec types in uapi headers ALSA: Deprecate CLOCK_REALTIME timestamps
Baolin Wang (2): ALSA: Replace timespec with timespec64 ALSA: Avoid using timespec for struct snd_ctl_elem_value
include/sound/asound.h | 8 +++++ include/sound/pcm.h | 22 +++++++----- include/sound/timer.h | 4 +-- include/uapi/sound/asound.h | 53 +++++++++++++++++++++-------- sound/core/Kconfig | 11 ++++++ sound/core/compat.h | 11 ++++++ sound/core/pcm.c | 3 ++ sound/core/pcm_compat.c | 70 ++++++++++++++++++++++----------------- sound/core/pcm_lib.c | 36 ++++++++++++-------- sound/core/pcm_native.c | 25 ++++++++++---- sound/core/rawmidi_compat.c | 12 +++---- sound/core/timer.c | 28 ++++++++-------- sound/core/timer_compat.c | 4 ++- sound/pci/hda/hda_controller.c | 14 +++++--- sound/soc/intel/skylake/skl-pcm.c | 4 +-- 15 files changed, 203 insertions(+), 102 deletions(-) create mode 100644 sound/core/compat.h
On Thu, Apr 26, 2018 at 3:30 PM, Jaroslav Kysela perex@perex.cz wrote:
Dne 26.4.2018 v 14:44 Arnd Bergmann napsal(a):
I've tried the suggestion from Jaroslaw, doing a minimal change to the UAPI headers to keep the existing binary interface. As he predicted, this is a much simpler set of kernel changes, but we will pay for that with added complexity in alsa-lib.
The first two patches in this series are taken from Baolin's patch set, with a small bugfix folded in to avoid a compile-time regression.
The other two patches are to redefine the UAPI and to deprecate the support for CLOCK_REALTIME time stamps, which we can no longer allow with user space that we expect to survive beyond 2038.
Overall, I'd still be happier with Baolin's approach since it allows us to keep compatiblity with CLOCK_REALTIME users and requires fewer changes in user space, but this would work as well.
Hi Arnd,
Thanks for your work. I proposed a bit different implementation. Example:
struct snd_example { struct snd_native_timespec tstamp; .... u64 tstamp_sec64; /* use the reserved[] array for this */ };
So tstamp contains the current 32-bit tv_sec/tv_nsec and the full 64-bit value is in tstamp_sec64. In this way, we can transfer any type of the timespec64 values and it's backward compatible to retain the binary compatibility. The protocol versions should be increased to let the userspace know about the new 64-bit fields.
Right, I went in a slightly different way since the intention was to keep the interface simple. I think we can either force the use of monotonic times or extend it to 64-bit CLOCK_REALTIME stamps, but the monotonic stamps seem much better for multiple reasons (i.e. skipping) if you want to avoid introducing new ioctls.
The added complexity of having two timestamps in a single structure means we don't end up with much simpler code that what Baolin proposed, which mostly just moves the existing compat_ioctl() to the native 32-bit handler but not add anything new that requires library changes.
His tread patch and my mmap patch both do add some complexity but then we also need some of that with your suggestions for tread.
The timer read protocol must be updated, because the stream will change, so I am fine to add new ioctl (like originally proposed).
With forced monotonic times, we can skip that update and keep using the existing stream format.
The alsa-lib defines timespec only if posix defines are not set so glibc's time.h does not define the timespec structure - it may be improved.
Yes, we definitely need to improve that, since any application that relies on the timespec definition to come from alsa would otherwise get a structure with a 64-bit tv_sec but incorrect padding on tv_nsec (no padding on i386, padding on the wrong side for big-endian architectures).
One way out would be to define snd_timestamp_t and snd_htimestamp_t in terms of snd_monotonic_timestamp from the kernel header and let it still have the traditional layout even for applications built with 64-bit time_t.
The downside is again that applications may break when they cast between snd_htimestamp_t and timespec pointers.
Arnd
participants (2)
-
Arnd Bergmann -
Jaroslav Kysela