Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1736384 > unrolled thread

[RFC PATCH 0/7] Fix year 2038 issue for sound subsystem

Started byBaolin Wang <baolin.wang@linaro.org>
First post2017-09-21 08:20 +0200
Last post2017-09-22 11:20 +0200
Articles 7 — 5 participants

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH 0/7] Fix year 2038 issue for sound subsystem Baolin Wang <baolin.wang@linaro.org> - 2017-09-21 08:20 +0200
    [RFC PATCH 1/7] sound: Replace timespec with timespec64 Baolin Wang <baolin.wang@linaro.org> - 2017-09-21 08:20 +0200
      Re: [RFC PATCH 1/7] sound: Replace timespec with timespec64 Arnd Bergmann <arnd@arndb.de> - 2017-09-21 12:10 +0200
    Re: [RFC PATCH 0/7] Fix year 2038 issue for sound subsystem Takashi Sakamoto <o-takashi@sakamocchi.jp> - 2017-09-22 06:10 +0200
      Re: [RFC PATCH 0/7] Fix year 2038 issue for sound subsystem Baolin Wang <baolin.wang@linaro.org> - 2017-09-22 07:40 +0200
      Re: [RFC PATCH 0/7] Fix year 2038 issue for sound subsystem Takashi Iwai <tiwai@suse.de> - 2017-09-22 11:20 +0200
      Re: [RFC PATCH 0/7] Fix year 2038 issue for sound subsystem Mark Brown <broonie@kernel.org> - 2017-09-22 11:20 +0200

#1736384 — [RFC PATCH 0/7] Fix year 2038 issue for sound subsystem

FromBaolin Wang <baolin.wang@linaro.org>
Date2017-09-21 08:20 +0200
Subject[RFC PATCH 0/7] Fix year 2038 issue for sound subsystem
Message-ID<us305-5ww-3@gated-at.bofh.it>
Since many structures will use timespec type variables to record time stamp
in uapi/asound.h, which are not year 2038 safe on 32bit system. This patchset
tries to introduce new structures removing timespec type to compatible native
mode and compat mode.

Moreover this patchset also converts the internal structrures to use timespec64
type and related APIs. 

Baolin Wang (7):
  sound: Replace timespec with timespec64
  sound: core: Avoid using timespec for struct snd_pcm_status
  sound: core: Avoid using timespec for struct snd_pcm_sync_ptr
  sound: core: Avoid using timespec for struct snd_rawmidi_status
  sound: core: Avoid using timespec for struct snd_timer_status
  uapi: sound: Avoid using timespec for struct snd_ctl_elem_value
  sound: core: Avoid using timespec for struct snd_timer_tread

 include/sound/pcm.h               |  113 ++++++++-
 include/sound/timer.h             |    4 +-
 include/uapi/sound/asound.h       |   15 +-
 sound/core/pcm.c                  |   14 +-
 sound/core/pcm_compat.c           |  466 +++++++++++++++++++++++++++++--------
 sound/core/pcm_lib.c              |   33 +--
 sound/core/pcm_native.c           |  227 ++++++++++++++----
 sound/core/rawmidi.c              |   74 +++++-
 sound/core/rawmidi_compat.c       |   90 +++++--
 sound/core/timer.c                |  247 ++++++++++++++++----
 sound/core/timer_compat.c         |   25 +-
 sound/pci/hda/hda_controller.c    |   10 +-
 sound/soc/intel/skylake/skl-pcm.c |    4 +-
 13 files changed, 1046 insertions(+), 276 deletions(-)

-- 
1.7.9.5

[toc] | [next] | [standalone]

#1736385 — [RFC PATCH 1/7] sound: Replace timespec with timespec64

FromBaolin Wang <baolin.wang@linaro.org>
Date2017-09-21 08:20 +0200
Subject[RFC PATCH 1/7] sound: Replace timespec with timespec64
Message-ID<us305-5ww-15@gated-at.bofh.it>
In reply to#1736384
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>
---
 include/sound/pcm.h               |   18 +++++++++---------
 include/sound/timer.h             |    4 ++--
 sound/core/pcm_lib.c              |   30 +++++++++++++++++-------------
 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 ++--
 7 files changed, 57 insertions(+), 49 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 24febf9..cd1ecd6 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,
@@ -343,7 +343,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;
@@ -419,7 +419,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 -- */
@@ -1167,22 +1167,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 c4d76ff..c196c07 100644
--- a/include/sound/timer.h
+++ b/include/sound/timer.h
@@ -102,7 +102,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;
@@ -126,7 +126,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_lib.c b/sound/core/pcm_lib.c
index a93a423..5ca9dc3 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,16 +250,16 @@ 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);
 	}
-	runtime->status->audio_tstamp = *audio_tstamp;
-	runtime->status->tstamp = *curr_tstamp;
+	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;
 }
 
@@ -268,8 +272,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;
@@ -292,9 +296,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 2fec2fe..60bc303 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -881,12 +881,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)
@@ -899,8 +899,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 6cdd04a..f44d702 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;
@@ -408,12 +408,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;
@@ -957,7 +957,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;
@@ -1251,7 +1251,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;
@@ -1265,7 +1265,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);
@@ -1288,7 +1288,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));
@@ -1301,14 +1301,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;
@@ -1322,14 +1322,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++;
@@ -1808,7 +1808,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 d1eb148..c3e4516 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 2b1e513..9f905c4 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -1141,7 +1141,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)
 {
@@ -1159,7 +1159,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 */
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1736515 — Re: [RFC PATCH 1/7] sound: Replace timespec with timespec64

FromArnd Bergmann <arnd@arndb.de>
Date2017-09-21 12:10 +0200
SubjectRe: [RFC PATCH 1/7] sound: Replace timespec with timespec64
Message-ID<us6AG-7W2-9@gated-at.bofh.it>
In reply to#1736385
On Thu, Sep 21, 2017 at 8:18 AM, Baolin Wang <baolin.wang@linaro.org> wrote:
> 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>

Looks good to me. This could perhaps be split up further, but it
seems small enough to get merged as a single patch.

     Arnd

[toc] | [prev] | [next] | [standalone]

#1737176

FromTakashi Sakamoto <o-takashi@sakamocchi.jp>
Date2017-09-22 06:10 +0200
Message-ID<usnrQ-1uE-7@gated-at.bofh.it>
In reply to#1736384
Hi,

On Sep 21 2017 15:18, Baolin Wang wrote:
> Since many structures will use timespec type variables to record time stamp
> in uapi/asound.h, which are not year 2038 safe on 32bit system. This patchset
> tries to introduce new structures removing timespec type to compatible native
> mode and compat mode.
> 
> Moreover this patchset also converts the internal structrures to use timespec64
> type and related APIs.
> 
> Baolin Wang (7):
>    sound: Replace timespec with timespec64
>    sound: core: Avoid using timespec for struct snd_pcm_status
>    sound: core: Avoid using timespec for struct snd_pcm_sync_ptr
>    sound: core: Avoid using timespec for struct snd_rawmidi_status
>    sound: core: Avoid using timespec for struct snd_timer_status
>    uapi: sound: Avoid using timespec for struct snd_ctl_elem_value
>    sound: core: Avoid using timespec for struct snd_timer_tread
> 
>   include/sound/pcm.h               |  113 ++++++++-
>   include/sound/timer.h             |    4 +-
>   include/uapi/sound/asound.h       |   15 +-
>   sound/core/pcm.c                  |   14 +-
>   sound/core/pcm_compat.c           |  466 +++++++++++++++++++++++++++++--------
>   sound/core/pcm_lib.c              |   33 +--
>   sound/core/pcm_native.c           |  227 ++++++++++++++----
>   sound/core/rawmidi.c              |   74 +++++-
>   sound/core/rawmidi_compat.c       |   90 +++++--
>   sound/core/timer.c                |  247 ++++++++++++++++----
>   sound/core/timer_compat.c         |   25 +-
>   sound/pci/hda/hda_controller.c    |   10 +-
>   sound/soc/intel/skylake/skl-pcm.c |    4 +-
>   13 files changed, 1046 insertions(+), 276 deletions(-)

I'm a minor Takashi in this subsystem and not those who you'd like to
talk about this issue. But I have interests in it and would like to
assist you, as long as I can do for it.

As a nitpicking, your patchset brings compilation error at
configurations for x86, and x86-64 with x32 ABI support.


## x86-64 architecture and amd64 ABI support
CONFIG_64BIT=y
CONFIG_X86_64=y

Success.


## x86-64 architecture and amd64/x32 ABI support
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86_X32=y

```
sound/core/timer_compat.c:124:54: error: array type has incomplete 
element type 'struct snd_timer_status32'
   SNDRV_TIMER_IOCTL_STATUS32 = _IOW('T', 0x14, struct snd_timer_status32),
```

This error comes from a commit 1229cccbefe7 ('sound: core: Avoid using 
timespec for struct snd_timer_status').

```
sound/core/pcm_compat.c: In function 'snd_pcm_ioctl_sync_ptr_compat':
sound/core/pcm_compat.c:623:9: error: assignment from incompatible 
pointer type [-Werror=incompatible-pointer-types]
   status = runtime->status;
          ^
sound/core/pcm_compat.c: In function 'snd_pcm_ioctl_sync_ptr_x32':
sound/core/pcm_compat.c:711:9: error: assignment from incompatible 
pointer type [-Werror=incompatible-pointer-types]
   status = runtime->status;
```

This error comes from a commit 947c463adc00('sound: core: Avoid using 
timespec
for struct snd_pcm_status').


## x86 architecture and i386 ABI support
CONFIG_X86_32=y

```
sound/core/pcm_native.c: In function 'snd_pcm_common_ioctl':
sound/core/pcm_native.c:3065:2: error: duplicate case value
   case SNDRV_PCM_IOCTL_SYNC_PTR64:
   ^~~~
sound/core/pcm_native.c:3062:2: error: previously used here
   case SNDRV_PCM_IOCTL_SYNC_PTR32:

```

This error comes from a commit c0513348a7b39 ('sound: core: Avoid using
timespec for struct snd_pcm_sync_ptr').


Your patchset brought conflicts to 'for-next' branch in a repository
which Iwai-san maintains[1]. I rebased your patchset on a commit 
729fbfc92a45 ('ALSA: line6: add support for POD HD DESKTOP') which is a 
HEAD of 'for-next' branch and pushed into my repository on github[2].


I respect your work for this issue, however it's better to check whether
your patchset is buildable or not on major configurations before
posting.


I note that at a development period for v4.5 kernel, ALSA developers
(mainly Iwai-san) fixed x32 ABI compatibility bugs. Then I prepared for
a rough set of test for ioctl command[3] to check his work. The set will
partly help your work, I think (but it's really rough).

I need more time for reviewing. At least, this week is for recovery from
my tough work to rewrite aplay[4].


[1] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
[2] https://github.com/takaswie/sound/tree/topic/year2038-rfc1
[3] https://github.com/takaswie/alsa-ioctl-test/
[4] [alsa-devel] [RFCv2][PATCH 00/38] alsa-utils: axfer: rewrite aplay
http://mailman.alsa-project.org/pipermail/alsa-devel/2017-September/125574.html

Thanks

Takashi Sakamoto

[toc] | [prev] | [next] | [standalone]

#1737191

FromBaolin Wang <baolin.wang@linaro.org>
Date2017-09-22 07:40 +0200
Message-ID<usoQW-2fe-3@gated-at.bofh.it>
In reply to#1737176
Hi Takashi,

On 22 September 2017 at 12:07, Takashi Sakamoto <o-takashi@sakamocchi.jp> wrote:
> Hi,
>
>
> On Sep 21 2017 15:18, Baolin Wang wrote:
>>
>> Since many structures will use timespec type variables to record time
>> stamp
>> in uapi/asound.h, which are not year 2038 safe on 32bit system. This
>> patchset
>> tries to introduce new structures removing timespec type to compatible
>> native
>> mode and compat mode.
>>
>> Moreover this patchset also converts the internal structrures to use
>> timespec64
>> type and related APIs.
>>
>> Baolin Wang (7):
>>    sound: Replace timespec with timespec64
>>    sound: core: Avoid using timespec for struct snd_pcm_status
>>    sound: core: Avoid using timespec for struct snd_pcm_sync_ptr
>>    sound: core: Avoid using timespec for struct snd_rawmidi_status
>>    sound: core: Avoid using timespec for struct snd_timer_status
>>    uapi: sound: Avoid using timespec for struct snd_ctl_elem_value
>>    sound: core: Avoid using timespec for struct snd_timer_tread
>>
>>   include/sound/pcm.h               |  113 ++++++++-
>>   include/sound/timer.h             |    4 +-
>>   include/uapi/sound/asound.h       |   15 +-
>>   sound/core/pcm.c                  |   14 +-
>>   sound/core/pcm_compat.c           |  466
>> +++++++++++++++++++++++++++++--------
>>   sound/core/pcm_lib.c              |   33 +--
>>   sound/core/pcm_native.c           |  227 ++++++++++++++----
>>   sound/core/rawmidi.c              |   74 +++++-
>>   sound/core/rawmidi_compat.c       |   90 +++++--
>>   sound/core/timer.c                |  247 ++++++++++++++++----
>>   sound/core/timer_compat.c         |   25 +-
>>   sound/pci/hda/hda_controller.c    |   10 +-
>>   sound/soc/intel/skylake/skl-pcm.c |    4 +-
>>   13 files changed, 1046 insertions(+), 276 deletions(-)
>
>
> I'm a minor Takashi in this subsystem and not those who you'd like to
> talk about this issue. But I have interests in it and would like to
> assist you, as long as I can do for it.

Thanks a lot.

>
> As a nitpicking, your patchset brings compilation error at
> configurations for x86, and x86-64 with x32 ABI support.
>
>
> ## x86-64 architecture and amd64 ABI support
> CONFIG_64BIT=y
> CONFIG_X86_64=y
>
> Success.
>
>
> ## x86-64 architecture and amd64/x32 ABI support
> CONFIG_64BIT=y
> CONFIG_X86_64=y
> CONFIG_X86_X32=y
>
> ```
> sound/core/timer_compat.c:124:54: error: array type has incomplete element
> type 'struct snd_timer_status32'
>   SNDRV_TIMER_IOCTL_STATUS32 = _IOW('T', 0x14, struct snd_timer_status32),
> ```
>
> This error comes from a commit 1229cccbefe7 ('sound: core: Avoid using
> timespec for struct snd_timer_status').
>
> ```
> sound/core/pcm_compat.c: In function 'snd_pcm_ioctl_sync_ptr_compat':
> sound/core/pcm_compat.c:623:9: error: assignment from incompatible pointer
> type [-Werror=incompatible-pointer-types]
>   status = runtime->status;
>          ^
> sound/core/pcm_compat.c: In function 'snd_pcm_ioctl_sync_ptr_x32':
> sound/core/pcm_compat.c:711:9: error: assignment from incompatible pointer
> type [-Werror=incompatible-pointer-types]
>   status = runtime->status;
> ```
>
> This error comes from a commit 947c463adc00('sound: core: Avoid using
> timespec
> for struct snd_pcm_status').
>
>
> ## x86 architecture and i386 ABI support
> CONFIG_X86_32=y
>
> ```
> sound/core/pcm_native.c: In function 'snd_pcm_common_ioctl':
> sound/core/pcm_native.c:3065:2: error: duplicate case value
>   case SNDRV_PCM_IOCTL_SYNC_PTR64:
>   ^~~~
> sound/core/pcm_native.c:3062:2: error: previously used here
>   case SNDRV_PCM_IOCTL_SYNC_PTR32:
>
> ```
>
> This error comes from a commit c0513348a7b39 ('sound: core: Avoid using
> timespec for struct snd_pcm_sync_ptr').
>
>
> Your patchset brought conflicts to 'for-next' branch in a repository
> which Iwai-san maintains[1]. I rebased your patchset on a commit
> 729fbfc92a45 ('ALSA: line6: add support for POD HD DESKTOP') which is a HEAD
> of 'for-next' branch and pushed into my repository on github[2].
>
>
> I respect your work for this issue, however it's better to check whether
> your patchset is buildable or not on major configurations before
> posting.

Sorry for the building errors, since I can not build CONFIG_COMPAT
mode on my arm32 platform. But I will try to fix these build errors in
next version. This RFC patchset, I just want to show how to fix the
y2038 issue and to see if it is on the correct way. Sorry for the
building errors again.

>
> I note that at a development period for v4.5 kernel, ALSA developers
> (mainly Iwai-san) fixed x32 ABI compatibility bugs. Then I prepared for
> a rough set of test for ioctl command[3] to check his work. The set will
> partly help your work, I think (but it's really rough).

Ah, thanks.

>
> I need more time for reviewing. At least, this week is for recovery from
> my tough work to rewrite aplay[4].

Understood. Very appreciated for your comments.

>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> [2] https://github.com/takaswie/sound/tree/topic/year2038-rfc1
> [3] https://github.com/takaswie/alsa-ioctl-test/
> [4] [alsa-devel] [RFCv2][PATCH 00/38] alsa-utils: axfer: rewrite aplay
> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-September/125574.html
>
> Thanks
>
> Takashi Sakamoto



-- 
Baolin.wang
Best Regards

[toc] | [prev] | [next] | [standalone]

#1737316

FromTakashi Iwai <tiwai@suse.de>
Date2017-09-22 11:20 +0200
Message-ID<usshP-4kK-1@gated-at.bofh.it>
In reply to#1737176
On Fri, 22 Sep 2017 11:15:05 +0200,
Mark Brown wrote:
> 
> On Fri, Sep 22, 2017 at 01:07:37PM +0900, Takashi Sakamoto wrote:
> 
> > I note that at a development period for v4.5 kernel, ALSA developers
> > (mainly Iwai-san) fixed x32 ABI compatibility bugs. Then I prepared for
> > a rough set of test for ioctl command[3] to check his work. The set will
> > partly help your work, I think (but it's really rough).
> 
> Might it be worth trying to get these added to kselftest?  Seems like
> the sort of thing that fits well there and it'd make it more
> discoverable.

Yes, that sounds like a sensible option.
Some ioctl sanity checks can be done well without hardware, too.


thanks,

Takashi

[toc] | [prev] | [next] | [standalone]

#1737322

FromMark Brown <broonie@kernel.org>
Date2017-09-22 11:20 +0200
Message-ID<usshP-4kK-3@gated-at.bofh.it>
In reply to#1737176

[Multipart message — attachments visible in raw view] — view raw

On Fri, Sep 22, 2017 at 01:07:37PM +0900, Takashi Sakamoto wrote:

> I note that at a development period for v4.5 kernel, ALSA developers
> (mainly Iwai-san) fixed x32 ABI compatibility bugs. Then I prepared for
> a rough set of test for ioctl command[3] to check his work. The set will
> partly help your work, I think (but it's really rough).

Might it be worth trying to get these added to kselftest?  Seems like
the sort of thing that fits well there and it'd make it more
discoverable.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web