* [PATCH 0/7] [media] y2038 conversion for subsystem
@ 2015-09-15 15:49 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
linux-samsung-soc, Arnd Bergmann
Hi everyone,
This is a conversion of all subsystem-wide v4l2 code to avoid the
use of types based on time_t. The first five patches should all
be harmless and obvious, so they can get applied for 4.3 after
normal review.
The last two patches are marked RFC for now because their possible
impact on the user space ABI and to decide if this is the best
approach or whether we should instead introduce extra code in
the kernel to handle modified user space.
There are a few device drivers beyond this series that rely on
time_t/timeval/timespec internally, but they are all easy to fix
and can be taken care of later.
Arnd
Arnd Bergmann (7):
[media] dvb: use ktime_t for internal timeout
[media] dvb: remove unused systime() function
[media] dvb: don't use 'time_t' in event ioctl
[media] exynos4-is: use monotonic timestamps as advertized
[media] use v4l2_get_timestamp where possible
[RFC] [media]: v4l2: introduce v4l2_timeval
[RFC] [media] introduce v4l2_timespec type for timestamps
drivers/media/dvb-core/demux.h | 2 +-
drivers/media/dvb-core/dmxdev.c | 2 +-
drivers/media/dvb-core/dvb_demux.c | 17 ++++++-----------
drivers/media/dvb-core/dvb_demux.h | 4 ++--
drivers/media/dvb-core/dvb_net.c | 2 +-
drivers/media/dvb-frontends/dibx000_common.c | 10 ----------
drivers/media/dvb-frontends/dibx000_common.h | 2 --
drivers/media/pci/bt8xx/bttv-driver.c | 7 ++-----
drivers/media/pci/cx18/cx18-mailbox.c | 2 +-
drivers/media/pci/meye/meye.h | 2 +-
drivers/media/pci/zoran/zoran.h | 2 +-
drivers/media/platform/coda/coda.h | 2 +-
drivers/media/platform/exynos4-is/fimc-capture.c | 8 +-------
drivers/media/platform/exynos4-is/fimc-lite.c | 7 +------
drivers/media/platform/omap/omap_vout.c | 4 ++--
drivers/media/platform/omap3isp/ispstat.c | 5 ++---
drivers/media/platform/omap3isp/ispstat.h | 2 +-
drivers/media/platform/s3c-camif/camif-capture.c | 8 +-------
drivers/media/platform/vim2m.c | 2 +-
drivers/media/platform/vivid/vivid-ctrls.c | 2 +-
drivers/media/usb/cpia2/cpia2.h | 2 +-
drivers/media/usb/cpia2/cpia2_v4l.c | 2 +-
drivers/media/usb/gspca/gspca.c | 6 +++---
drivers/media/usb/usbvision/usbvision.h | 2 +-
drivers/media/v4l2-core/v4l2-common.c | 6 +++---
drivers/media/v4l2-core/v4l2-event.c | 20 +++++++++++++-------
drivers/staging/media/omap4iss/iss_video.c | 5 +----
include/media/v4l2-common.h | 2 +-
include/media/videobuf-core.h | 2 +-
include/trace/events/v4l2.h | 12 ++++++++++--
include/uapi/linux/dvb/video.h | 3 ++-
include/uapi/linux/omap3isp.h | 2 +-
include/uapi/linux/videodev2.h | 16 ++++++++++++++--
33 files changed, 79 insertions(+), 93 deletions(-)
--
2.1.0.rc2
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 0/7] [media] y2038 conversion for subsystem
@ 2015-09-15 15:49 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-samsung-soc, Arnd Bergmann, Mauro Carvalho Chehab, y2038,
linux-api, linux-kernel
Hi everyone,
This is a conversion of all subsystem-wide v4l2 code to avoid the
use of types based on time_t. The first five patches should all
be harmless and obvious, so they can get applied for 4.3 after
normal review.
The last two patches are marked RFC for now because their possible
impact on the user space ABI and to decide if this is the best
approach or whether we should instead introduce extra code in
the kernel to handle modified user space.
There are a few device drivers beyond this series that rely on
time_t/timeval/timespec internally, but they are all easy to fix
and can be taken care of later.
Arnd
Arnd Bergmann (7):
[media] dvb: use ktime_t for internal timeout
[media] dvb: remove unused systime() function
[media] dvb: don't use 'time_t' in event ioctl
[media] exynos4-is: use monotonic timestamps as advertized
[media] use v4l2_get_timestamp where possible
[RFC] [media]: v4l2: introduce v4l2_timeval
[RFC] [media] introduce v4l2_timespec type for timestamps
drivers/media/dvb-core/demux.h | 2 +-
drivers/media/dvb-core/dmxdev.c | 2 +-
drivers/media/dvb-core/dvb_demux.c | 17 ++++++-----------
drivers/media/dvb-core/dvb_demux.h | 4 ++--
drivers/media/dvb-core/dvb_net.c | 2 +-
drivers/media/dvb-frontends/dibx000_common.c | 10 ----------
drivers/media/dvb-frontends/dibx000_common.h | 2 --
drivers/media/pci/bt8xx/bttv-driver.c | 7 ++-----
drivers/media/pci/cx18/cx18-mailbox.c | 2 +-
drivers/media/pci/meye/meye.h | 2 +-
drivers/media/pci/zoran/zoran.h | 2 +-
drivers/media/platform/coda/coda.h | 2 +-
drivers/media/platform/exynos4-is/fimc-capture.c | 8 +-------
drivers/media/platform/exynos4-is/fimc-lite.c | 7 +------
drivers/media/platform/omap/omap_vout.c | 4 ++--
drivers/media/platform/omap3isp/ispstat.c | 5 ++---
drivers/media/platform/omap3isp/ispstat.h | 2 +-
drivers/media/platform/s3c-camif/camif-capture.c | 8 +-------
drivers/media/platform/vim2m.c | 2 +-
drivers/media/platform/vivid/vivid-ctrls.c | 2 +-
drivers/media/usb/cpia2/cpia2.h | 2 +-
drivers/media/usb/cpia2/cpia2_v4l.c | 2 +-
drivers/media/usb/gspca/gspca.c | 6 +++---
drivers/media/usb/usbvision/usbvision.h | 2 +-
drivers/media/v4l2-core/v4l2-common.c | 6 +++---
drivers/media/v4l2-core/v4l2-event.c | 20 +++++++++++++-------
drivers/staging/media/omap4iss/iss_video.c | 5 +----
include/media/v4l2-common.h | 2 +-
include/media/videobuf-core.h | 2 +-
include/trace/events/v4l2.h | 12 ++++++++++--
include/uapi/linux/dvb/video.h | 3 ++-
include/uapi/linux/omap3isp.h | 2 +-
include/uapi/linux/videodev2.h | 16 ++++++++++++++--
33 files changed, 79 insertions(+), 93 deletions(-)
--
2.1.0.rc2
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 1/7] [media] dvb: use ktime_t for internal timeout
2015-09-15 15:49 ` Arnd Bergmann
@ 2015-09-15 15:49 ` Arnd Bergmann
-1 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
linux-samsung-soc, Arnd Bergmann
The dvb demuxer code uses a 'struct timespec' to pass a timeout
as absolute time. This will cause problems on 32-bit architectures
in 2038 when time_t overflows, and it is racy with a concurrent
settimeofday() call.
This patch changes the code to use ktime_get() instead, using
the monotonic time base to avoid both the race and the overflow.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/dvb-core/demux.h | 2 +-
drivers/media/dvb-core/dmxdev.c | 2 +-
drivers/media/dvb-core/dvb_demux.c | 17 ++++++-----------
drivers/media/dvb-core/dvb_demux.h | 4 ++--
drivers/media/dvb-core/dvb_net.c | 2 +-
5 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/drivers/media/dvb-core/demux.h b/drivers/media/dvb-core/demux.h
index 833191bcd810..d8e2b1213bef 100644
--- a/drivers/media/dvb-core/demux.h
+++ b/drivers/media/dvb-core/demux.h
@@ -92,7 +92,7 @@ struct dmx_ts_feed {
int type,
enum dmx_ts_pes pes_type,
size_t circular_buffer_size,
- struct timespec timeout);
+ ktime_t timeout);
int (*start_filtering) (struct dmx_ts_feed* feed);
int (*stop_filtering) (struct dmx_ts_feed* feed);
};
diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index d0e3f9d85f34..0d20b379eeec 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -558,7 +558,7 @@ static int dvb_dmxdev_start_feed(struct dmxdev *dmxdev,
struct dmxdev_filter *filter,
struct dmxdev_feed *feed)
{
- struct timespec timeout = { 0 };
+ ktime_t timeout = ktime_set(0, 0);
struct dmx_pes_filter_params *para = &filter->params.pes;
dmx_output_t otype;
int ret;
diff --git a/drivers/media/dvb-core/dvb_demux.c b/drivers/media/dvb-core/dvb_demux.c
index 6c7ff0cdcd32..d83dd0eb5757 100644
--- a/drivers/media/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb-core/dvb_demux.c
@@ -399,28 +399,23 @@ static void dvb_dmx_swfilter_packet(struct dvb_demux *demux, const u8 *buf)
int dvr_done = 0;
if (dvb_demux_speedcheck) {
- struct timespec cur_time, delta_time;
+ ktime_t cur_time;
u64 speed_bytes, speed_timedelta;
demux->speed_pkts_cnt++;
/* show speed every SPEED_PKTS_INTERVAL packets */
if (!(demux->speed_pkts_cnt % SPEED_PKTS_INTERVAL)) {
- cur_time = current_kernel_time();
+ cur_time = ktime_get();
- if (demux->speed_last_time.tv_sec != 0 &&
- demux->speed_last_time.tv_nsec != 0) {
- delta_time = timespec_sub(cur_time,
- demux->speed_last_time);
+ if (ktime_to_ns(demux->speed_last_time) == 0) {
speed_bytes = (u64)demux->speed_pkts_cnt
* 188 * 8;
/* convert to 1024 basis */
speed_bytes = 1000 * div64_u64(speed_bytes,
1024);
- speed_timedelta =
- (u64)timespec_to_ns(&delta_time);
- speed_timedelta = div64_u64(speed_timedelta,
- 1000000); /* nsec -> usec */
+ speed_timedelta = ktime_ms_delta(cur_time,
+ demux->speed_last_time);
printk(KERN_INFO "TS speed %llu Kbits/sec \n",
div64_u64(speed_bytes,
speed_timedelta));
@@ -667,7 +662,7 @@ out:
static int dmx_ts_feed_set(struct dmx_ts_feed *ts_feed, u16 pid, int ts_type,
enum dmx_ts_pes pes_type,
- size_t circular_buffer_size, struct timespec timeout)
+ size_t circular_buffer_size, ktime_t timeout)
{
struct dvb_demux_feed *feed = (struct dvb_demux_feed *)ts_feed;
struct dvb_demux *demux = feed->demux;
diff --git a/drivers/media/dvb-core/dvb_demux.h b/drivers/media/dvb-core/dvb_demux.h
index ae7fc33c3231..5ed3cab4ad28 100644
--- a/drivers/media/dvb-core/dvb_demux.h
+++ b/drivers/media/dvb-core/dvb_demux.h
@@ -83,7 +83,7 @@ struct dvb_demux_feed {
u8 *buffer;
int buffer_size;
- struct timespec timeout;
+ ktime_t timeout;
struct dvb_demux_filter *filter;
int ts_type;
@@ -134,7 +134,7 @@ struct dvb_demux {
uint8_t *cnt_storage; /* for TS continuity check */
- struct timespec speed_last_time; /* for TS speed check */
+ ktime_t speed_last_time; /* for TS speed check */
uint32_t speed_pkts_cnt; /* for TS speed check */
};
diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
index b81e026edab3..df3ba15c007e 100644
--- a/drivers/media/dvb-core/dvb_net.c
+++ b/drivers/media/dvb-core/dvb_net.c
@@ -998,7 +998,7 @@ static int dvb_net_feed_start(struct net_device *dev)
netdev_dbg(dev, "start filtering\n");
priv->secfeed->start_filtering(priv->secfeed);
} else if (priv->feedtype == DVB_NET_FEEDTYPE_ULE) {
- struct timespec timeout = { 0, 10000000 }; // 10 msec
+ ktime_t timeout = ns_to_ktime(10 * NSEC_PER_MSEC);
/* we have payloads encapsulated in TS */
netdev_dbg(dev, "alloc tsfeed\n");
--
2.1.0.rc2
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 1/7] [media] dvb: use ktime_t for internal timeout
@ 2015-09-15 15:49 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-samsung-soc, Arnd Bergmann, Mauro Carvalho Chehab, y2038,
linux-api, linux-kernel
The dvb demuxer code uses a 'struct timespec' to pass a timeout
as absolute time. This will cause problems on 32-bit architectures
in 2038 when time_t overflows, and it is racy with a concurrent
settimeofday() call.
This patch changes the code to use ktime_get() instead, using
the monotonic time base to avoid both the race and the overflow.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/dvb-core/demux.h | 2 +-
drivers/media/dvb-core/dmxdev.c | 2 +-
drivers/media/dvb-core/dvb_demux.c | 17 ++++++-----------
drivers/media/dvb-core/dvb_demux.h | 4 ++--
drivers/media/dvb-core/dvb_net.c | 2 +-
5 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/drivers/media/dvb-core/demux.h b/drivers/media/dvb-core/demux.h
index 833191bcd810..d8e2b1213bef 100644
--- a/drivers/media/dvb-core/demux.h
+++ b/drivers/media/dvb-core/demux.h
@@ -92,7 +92,7 @@ struct dmx_ts_feed {
int type,
enum dmx_ts_pes pes_type,
size_t circular_buffer_size,
- struct timespec timeout);
+ ktime_t timeout);
int (*start_filtering) (struct dmx_ts_feed* feed);
int (*stop_filtering) (struct dmx_ts_feed* feed);
};
diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index d0e3f9d85f34..0d20b379eeec 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -558,7 +558,7 @@ static int dvb_dmxdev_start_feed(struct dmxdev *dmxdev,
struct dmxdev_filter *filter,
struct dmxdev_feed *feed)
{
- struct timespec timeout = { 0 };
+ ktime_t timeout = ktime_set(0, 0);
struct dmx_pes_filter_params *para = &filter->params.pes;
dmx_output_t otype;
int ret;
diff --git a/drivers/media/dvb-core/dvb_demux.c b/drivers/media/dvb-core/dvb_demux.c
index 6c7ff0cdcd32..d83dd0eb5757 100644
--- a/drivers/media/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb-core/dvb_demux.c
@@ -399,28 +399,23 @@ static void dvb_dmx_swfilter_packet(struct dvb_demux *demux, const u8 *buf)
int dvr_done = 0;
if (dvb_demux_speedcheck) {
- struct timespec cur_time, delta_time;
+ ktime_t cur_time;
u64 speed_bytes, speed_timedelta;
demux->speed_pkts_cnt++;
/* show speed every SPEED_PKTS_INTERVAL packets */
if (!(demux->speed_pkts_cnt % SPEED_PKTS_INTERVAL)) {
- cur_time = current_kernel_time();
+ cur_time = ktime_get();
- if (demux->speed_last_time.tv_sec != 0 &&
- demux->speed_last_time.tv_nsec != 0) {
- delta_time = timespec_sub(cur_time,
- demux->speed_last_time);
+ if (ktime_to_ns(demux->speed_last_time) == 0) {
speed_bytes = (u64)demux->speed_pkts_cnt
* 188 * 8;
/* convert to 1024 basis */
speed_bytes = 1000 * div64_u64(speed_bytes,
1024);
- speed_timedelta =
- (u64)timespec_to_ns(&delta_time);
- speed_timedelta = div64_u64(speed_timedelta,
- 1000000); /* nsec -> usec */
+ speed_timedelta = ktime_ms_delta(cur_time,
+ demux->speed_last_time);
printk(KERN_INFO "TS speed %llu Kbits/sec \n",
div64_u64(speed_bytes,
speed_timedelta));
@@ -667,7 +662,7 @@ out:
static int dmx_ts_feed_set(struct dmx_ts_feed *ts_feed, u16 pid, int ts_type,
enum dmx_ts_pes pes_type,
- size_t circular_buffer_size, struct timespec timeout)
+ size_t circular_buffer_size, ktime_t timeout)
{
struct dvb_demux_feed *feed = (struct dvb_demux_feed *)ts_feed;
struct dvb_demux *demux = feed->demux;
diff --git a/drivers/media/dvb-core/dvb_demux.h b/drivers/media/dvb-core/dvb_demux.h
index ae7fc33c3231..5ed3cab4ad28 100644
--- a/drivers/media/dvb-core/dvb_demux.h
+++ b/drivers/media/dvb-core/dvb_demux.h
@@ -83,7 +83,7 @@ struct dvb_demux_feed {
u8 *buffer;
int buffer_size;
- struct timespec timeout;
+ ktime_t timeout;
struct dvb_demux_filter *filter;
int ts_type;
@@ -134,7 +134,7 @@ struct dvb_demux {
uint8_t *cnt_storage; /* for TS continuity check */
- struct timespec speed_last_time; /* for TS speed check */
+ ktime_t speed_last_time; /* for TS speed check */
uint32_t speed_pkts_cnt; /* for TS speed check */
};
diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
index b81e026edab3..df3ba15c007e 100644
--- a/drivers/media/dvb-core/dvb_net.c
+++ b/drivers/media/dvb-core/dvb_net.c
@@ -998,7 +998,7 @@ static int dvb_net_feed_start(struct net_device *dev)
netdev_dbg(dev, "start filtering\n");
priv->secfeed->start_filtering(priv->secfeed);
} else if (priv->feedtype == DVB_NET_FEEDTYPE_ULE) {
- struct timespec timeout = { 0, 10000000 }; // 10 msec
+ ktime_t timeout = ns_to_ktime(10 * NSEC_PER_MSEC);
/* we have payloads encapsulated in TS */
netdev_dbg(dev, "alloc tsfeed\n");
--
2.1.0.rc2
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 2/7] [media] dvb: remove unused systime() function
2015-09-15 15:49 ` Arnd Bergmann
@ 2015-09-15 15:49 ` Arnd Bergmann
-1 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
linux-samsung-soc, Arnd Bergmann
The systime function uses struct timespec, which we want to stop
using in the kernel because it overflows in 2038. Fortunately,
this use in dibx000_common is in a function that is never called,
so we can just remove it.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/dvb-frontends/dibx000_common.c | 10 ----------
drivers/media/dvb-frontends/dibx000_common.h | 2 --
2 files changed, 12 deletions(-)
diff --git a/drivers/media/dvb-frontends/dibx000_common.c b/drivers/media/dvb-frontends/dibx000_common.c
index 43be7238311e..79cb007b401f 100644
--- a/drivers/media/dvb-frontends/dibx000_common.c
+++ b/drivers/media/dvb-frontends/dibx000_common.c
@@ -500,16 +500,6 @@ void dibx000_exit_i2c_master(struct dibx000_i2c_master *mst)
}
EXPORT_SYMBOL(dibx000_exit_i2c_master);
-
-u32 systime(void)
-{
- struct timespec t;
-
- t = current_kernel_time();
- return (t.tv_sec * 10000) + (t.tv_nsec / 100000);
-}
-EXPORT_SYMBOL(systime);
-
MODULE_AUTHOR("Patrick Boettcher <pboettcher@dibcom.fr>");
MODULE_DESCRIPTION("Common function the DiBcom demodulator family");
MODULE_LICENSE("GPL");
diff --git a/drivers/media/dvb-frontends/dibx000_common.h b/drivers/media/dvb-frontends/dibx000_common.h
index b538e0555c95..61f4152f24ee 100644
--- a/drivers/media/dvb-frontends/dibx000_common.h
+++ b/drivers/media/dvb-frontends/dibx000_common.h
@@ -47,8 +47,6 @@ extern void dibx000_exit_i2c_master(struct dibx000_i2c_master *mst);
extern void dibx000_reset_i2c_master(struct dibx000_i2c_master *mst);
extern int dibx000_i2c_set_speed(struct i2c_adapter *i2c_adap, u16 speed);
-extern u32 systime(void);
-
#define BAND_LBAND 0x01
#define BAND_UHF 0x02
#define BAND_VHF 0x04
--
2.1.0.rc2
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 2/7] [media] dvb: remove unused systime() function
@ 2015-09-15 15:49 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-samsung-soc, Arnd Bergmann, Mauro Carvalho Chehab, y2038,
linux-api, linux-kernel
The systime function uses struct timespec, which we want to stop
using in the kernel because it overflows in 2038. Fortunately,
this use in dibx000_common is in a function that is never called,
so we can just remove it.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/dvb-frontends/dibx000_common.c | 10 ----------
drivers/media/dvb-frontends/dibx000_common.h | 2 --
2 files changed, 12 deletions(-)
diff --git a/drivers/media/dvb-frontends/dibx000_common.c b/drivers/media/dvb-frontends/dibx000_common.c
index 43be7238311e..79cb007b401f 100644
--- a/drivers/media/dvb-frontends/dibx000_common.c
+++ b/drivers/media/dvb-frontends/dibx000_common.c
@@ -500,16 +500,6 @@ void dibx000_exit_i2c_master(struct dibx000_i2c_master *mst)
}
EXPORT_SYMBOL(dibx000_exit_i2c_master);
-
-u32 systime(void)
-{
- struct timespec t;
-
- t = current_kernel_time();
- return (t.tv_sec * 10000) + (t.tv_nsec / 100000);
-}
-EXPORT_SYMBOL(systime);
-
MODULE_AUTHOR("Patrick Boettcher <pboettcher@dibcom.fr>");
MODULE_DESCRIPTION("Common function the DiBcom demodulator family");
MODULE_LICENSE("GPL");
diff --git a/drivers/media/dvb-frontends/dibx000_common.h b/drivers/media/dvb-frontends/dibx000_common.h
index b538e0555c95..61f4152f24ee 100644
--- a/drivers/media/dvb-frontends/dibx000_common.h
+++ b/drivers/media/dvb-frontends/dibx000_common.h
@@ -47,8 +47,6 @@ extern void dibx000_exit_i2c_master(struct dibx000_i2c_master *mst);
extern void dibx000_reset_i2c_master(struct dibx000_i2c_master *mst);
extern int dibx000_i2c_set_speed(struct i2c_adapter *i2c_adap, u16 speed);
-extern u32 systime(void);
-
#define BAND_LBAND 0x01
#define BAND_UHF 0x02
#define BAND_VHF 0x04
--
2.1.0.rc2
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 3/7] [media] dvb: don't use 'time_t' in event ioctl
2015-09-15 15:49 ` Arnd Bergmann
@ 2015-09-15 15:49 ` Arnd Bergmann
-1 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
linux-samsung-soc, Arnd Bergmann
'struct video_event' is used for the VIDEO_GET_EVENT ioctl, implemented
by drivers/media/pci/ivtv/ivtv-ioctl.c and
drivers/media/pci/ttpci/av7110_av.c. The structure contains a 'time_t',
which will be redefined in the future to be 64-bit wide, causing an
incompatible ABI change for this ioctl.
As it turns out, neither of the drivers currently sets the timestamp
field, and it is presumably useless anyway because of the limited
resolutions (no sub-second times). This means we can simply change
the structure definition to use a 'long' instead of 'time_t' and
remain compatible with all existing user space binaries when time_t
gets changed.
If anybody ever starts using this field, they have to make sure not
to use 1970 based seconds in there, as those overflow in 2038.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/uapi/linux/dvb/video.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/dvb/video.h b/include/uapi/linux/dvb/video.h
index d3d14a59d2d5..6c7f9298d7c2 100644
--- a/include/uapi/linux/dvb/video.h
+++ b/include/uapi/linux/dvb/video.h
@@ -135,7 +135,8 @@ struct video_event {
#define VIDEO_EVENT_FRAME_RATE_CHANGED 2
#define VIDEO_EVENT_DECODER_STOPPED 3
#define VIDEO_EVENT_VSYNC 4
- __kernel_time_t timestamp;
+ /* unused, make sure to use atomic time for y2038 if it ever gets used */
+ long timestamp;
union {
video_size_t size;
unsigned int frame_rate; /* in frames per 1000sec */
--
2.1.0.rc2
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 3/7] [media] dvb: don't use 'time_t' in event ioctl
@ 2015-09-15 15:49 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-samsung-soc, Arnd Bergmann, Mauro Carvalho Chehab, y2038,
linux-api, linux-kernel
'struct video_event' is used for the VIDEO_GET_EVENT ioctl, implemented
by drivers/media/pci/ivtv/ivtv-ioctl.c and
drivers/media/pci/ttpci/av7110_av.c. The structure contains a 'time_t',
which will be redefined in the future to be 64-bit wide, causing an
incompatible ABI change for this ioctl.
As it turns out, neither of the drivers currently sets the timestamp
field, and it is presumably useless anyway because of the limited
resolutions (no sub-second times). This means we can simply change
the structure definition to use a 'long' instead of 'time_t' and
remain compatible with all existing user space binaries when time_t
gets changed.
If anybody ever starts using this field, they have to make sure not
to use 1970 based seconds in there, as those overflow in 2038.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/uapi/linux/dvb/video.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/dvb/video.h b/include/uapi/linux/dvb/video.h
index d3d14a59d2d5..6c7f9298d7c2 100644
--- a/include/uapi/linux/dvb/video.h
+++ b/include/uapi/linux/dvb/video.h
@@ -135,7 +135,8 @@ struct video_event {
#define VIDEO_EVENT_FRAME_RATE_CHANGED 2
#define VIDEO_EVENT_DECODER_STOPPED 3
#define VIDEO_EVENT_VSYNC 4
- __kernel_time_t timestamp;
+ /* unused, make sure to use atomic time for y2038 if it ever gets used */
+ long timestamp;
union {
video_size_t size;
unsigned int frame_rate; /* in frames per 1000sec */
--
2.1.0.rc2
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 4/7] [media] exynos4-is: use monotonic timestamps as advertized
2015-09-15 15:49 ` Arnd Bergmann
` (3 preceding siblings ...)
(?)
@ 2015-09-15 15:49 ` Arnd Bergmann
2015-09-16 8:55 ` Sylwester Nawrocki
-1 siblings, 1 reply; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
linux-samsung-soc, Arnd Bergmann
The exynos4 fimc capture driver claims to use monotonic
timestamps but calls ktime_get_real_ts(). This is both
an incorrect API use, and a bad idea because of the y2038
problem and the fact that the wall clock time is not reliable
for timestamps across suspend or settimeofday().
This changes the driver to use the normal v4l2_get_timestamp()
function like all other drivers.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/platform/exynos4-is/fimc-capture.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index cfebf292e15a..776ea6d78d03 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -183,8 +183,6 @@ void fimc_capture_irq_handler(struct fimc_dev *fimc, int deq_buf)
struct v4l2_subdev *csis = p->subdevs[IDX_CSIS];
struct fimc_frame *f = &cap->ctx->d_frame;
struct fimc_vid_buffer *v_buf;
- struct timeval *tv;
- struct timespec ts;
if (test_and_clear_bit(ST_CAPT_SHUT, &fimc->state)) {
wake_up(&fimc->irq_queue);
@@ -193,13 +191,9 @@ void fimc_capture_irq_handler(struct fimc_dev *fimc, int deq_buf)
if (!list_empty(&cap->active_buf_q) &&
test_bit(ST_CAPT_RUN, &fimc->state) && deq_buf) {
- ktime_get_real_ts(&ts);
-
v_buf = fimc_active_queue_pop(cap);
- tv = &v_buf->vb.v4l2_buf.timestamp;
- tv->tv_sec = ts.tv_sec;
- tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
+ v4l2_get_timestamp(&v_buf->vb.v4l2_buf.timestamp);
v_buf->vb.v4l2_buf.sequence = cap->frame_count++;
vb2_buffer_done(&v_buf->vb, VB2_BUF_STATE_DONE);
--
2.1.0.rc2
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 5/7] [media] use v4l2_get_timestamp where possible
2015-09-15 15:49 ` Arnd Bergmann
@ 2015-09-15 15:49 ` Arnd Bergmann
-1 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
linux-samsung-soc, Arnd Bergmann
This is a preparation for a change to the type of v4l2 timestamps.
v4l2_get_timestamp() is a helper function that reads the monotonic
time and stores it into a 'struct timeval'. Multiple drivers implement
the same thing themselves for historic reasons.
Changing them all to use v4l2_get_timestamp() is more consistent
and reduces the amount of code duplication, and most importantly
simplifies the following changes.
If desired, this patch can easily be split up into one patch per
driver.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/pci/bt8xx/bttv-driver.c | 5 +----
drivers/media/pci/cx18/cx18-mailbox.c | 2 +-
drivers/media/platform/exynos4-is/fimc-lite.c | 7 +------
drivers/media/platform/omap3isp/ispstat.c | 5 ++---
drivers/media/platform/omap3isp/ispstat.h | 2 +-
drivers/media/platform/s3c-camif/camif-capture.c | 8 +-------
drivers/media/usb/gspca/gspca.c | 4 ++--
drivers/staging/media/omap4iss/iss_video.c | 5 +----
8 files changed, 10 insertions(+), 28 deletions(-)
diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
index 3632958f2158..15a4ebc2844d 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -3625,13 +3625,10 @@ static void
bttv_irq_wakeup_vbi(struct bttv *btv, struct bttv_buffer *wakeup,
unsigned int state)
{
- struct timeval ts;
-
if (NULL == wakeup)
return;
- v4l2_get_timestamp(&ts);
- wakeup->vb.ts = ts;
+ v4l2_get_timestamp(&wakeup->vb.ts);
wakeup->vb.field_count = btv->field_count;
wakeup->vb.state = state;
wake_up(&wakeup->vb.done);
diff --git a/drivers/media/pci/cx18/cx18-mailbox.c b/drivers/media/pci/cx18/cx18-mailbox.c
index eabf00c6351b..1f8aa9a749a1 100644
--- a/drivers/media/pci/cx18/cx18-mailbox.c
+++ b/drivers/media/pci/cx18/cx18-mailbox.c
@@ -202,7 +202,7 @@ static void cx18_mdl_send_to_videobuf(struct cx18_stream *s,
}
if (dispatch) {
- vb_buf->vb.ts = ktime_to_timeval(ktime_get());
+ v4l2_get_timestamp(&vb_buf->vb.ts);
list_del(&vb_buf->vb.queue);
vb_buf->vb.state = VIDEOBUF_DONE;
wake_up(&vb_buf->vb.done);
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index ca6261a86a5f..459bc65b545d 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -254,8 +254,6 @@ static irqreturn_t flite_irq_handler(int irq, void *priv)
struct fimc_lite *fimc = priv;
struct flite_buffer *vbuf;
unsigned long flags;
- struct timeval *tv;
- struct timespec ts;
u32 intsrc;
spin_lock_irqsave(&fimc->slock, flags);
@@ -294,10 +292,7 @@ static irqreturn_t flite_irq_handler(int irq, void *priv)
test_bit(ST_FLITE_RUN, &fimc->state) &&
!list_empty(&fimc->active_buf_q)) {
vbuf = fimc_lite_active_queue_pop(fimc);
- ktime_get_ts(&ts);
- tv = &vbuf->vb.v4l2_buf.timestamp;
- tv->tv_sec = ts.tv_sec;
- tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
+ v4l2_get_timestamp(&vbuf->vb.v4l2_buf.timestamp);
vbuf->vb.v4l2_buf.sequence = fimc->frame_count++;
flite_hw_mask_dma_buffer(fimc, vbuf->index);
vb2_buffer_done(&vbuf->vb, VB2_BUF_STATE_DONE);
diff --git a/drivers/media/platform/omap3isp/ispstat.c b/drivers/media/platform/omap3isp/ispstat.c
index 20434e83e801..94d4c295d3d0 100644
--- a/drivers/media/platform/omap3isp/ispstat.c
+++ b/drivers/media/platform/omap3isp/ispstat.c
@@ -235,7 +235,7 @@ static int isp_stat_buf_queue(struct ispstat *stat)
if (!stat->active_buf)
return STAT_NO_BUF;
- ktime_get_ts(&stat->active_buf->ts);
+ v4l2_get_timestamp(&stat->active_buf->ts);
stat->active_buf->buf_size = stat->buf_size;
if (isp_stat_buf_check_magic(stat, stat->active_buf)) {
@@ -496,8 +496,7 @@ int omap3isp_stat_request_statistics(struct ispstat *stat,
return PTR_ERR(buf);
}
- data->ts.tv_sec = buf->ts.tv_sec;
- data->ts.tv_usec = buf->ts.tv_nsec / NSEC_PER_USEC;
+ data->ts = buf->ts;
data->config_counter = buf->config_counter;
data->frame_number = buf->frame_number;
data->buf_size = buf->buf_size;
diff --git a/drivers/media/platform/omap3isp/ispstat.h b/drivers/media/platform/omap3isp/ispstat.h
index b79380d83fcf..6d9b0244f320 100644
--- a/drivers/media/platform/omap3isp/ispstat.h
+++ b/drivers/media/platform/omap3isp/ispstat.h
@@ -39,7 +39,7 @@ struct ispstat_buffer {
struct sg_table sgt;
void *virt_addr;
dma_addr_t dma_addr;
- struct timespec ts;
+ struct timeval ts;
u32 buf_size;
u32 frame_number;
u16 config_counter;
diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
index 76e6289a5612..edf70725ecf3 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -328,23 +328,17 @@ irqreturn_t s3c_camif_irq_handler(int irq, void *priv)
!list_empty(&vp->active_buf_q)) {
unsigned int index;
struct camif_buffer *vbuf;
- struct timeval *tv;
- struct timespec ts;
/*
* Get previous DMA write buffer index:
* 0 => DMA buffer 0, 2;
* 1 => DMA buffer 1, 3.
*/
index = (CISTATUS_FRAMECNT(status) + 2) & 1;
-
- ktime_get_ts(&ts);
vbuf = camif_active_queue_peek(vp, index);
if (!WARN_ON(vbuf == NULL)) {
/* Dequeue a filled buffer */
- tv = &vbuf->vb.v4l2_buf.timestamp;
- tv->tv_sec = ts.tv_sec;
- tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
+ v4l2_get_timestamp(&vbuf->vb.v4l2_buf.timestamp);
vbuf->vb.v4l2_buf.sequence = vp->frame_sequence++;
vb2_buffer_done(&vbuf->vb, VB2_BUF_STATE_DONE);
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index e54cee856a80..af5cd8213e8b 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -436,7 +436,7 @@ void gspca_frame_add(struct gspca_dev *gspca_dev,
}
j = gspca_dev->fr_queue[i];
frame = &gspca_dev->frame[j];
- frame->v4l2_buf.timestamp = ktime_to_timeval(ktime_get());
+ v4l2_get_timestamp(&frame->v4l2_buf.timestamp);
frame->v4l2_buf.sequence = gspca_dev->sequence++;
gspca_dev->image = frame->data;
gspca_dev->image_len = 0;
@@ -1909,7 +1909,7 @@ static ssize_t dev_read(struct file *file, char __user *data,
}
/* get a frame */
- timestamp = ktime_to_timeval(ktime_get());
+ v4l2_get_timestamp(×tamp);
timestamp.tv_sec--;
n = 2;
for (;;) {
diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index 40405d8710a6..485a90ce12df 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -420,7 +420,6 @@ struct iss_buffer *omap4iss_video_buffer_next(struct iss_video *video)
enum iss_pipeline_state state;
struct iss_buffer *buf;
unsigned long flags;
- struct timespec ts;
spin_lock_irqsave(&video->qlock, flags);
if (WARN_ON(list_empty(&video->dmaqueue))) {
@@ -433,9 +432,7 @@ struct iss_buffer *omap4iss_video_buffer_next(struct iss_video *video)
list_del(&buf->list);
spin_unlock_irqrestore(&video->qlock, flags);
- ktime_get_ts(&ts);
- buf->vb.v4l2_buf.timestamp.tv_sec = ts.tv_sec;
- buf->vb.v4l2_buf.timestamp.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
+ v4l2_get_timestamp(&buf->vb.v4l2_buf.timestamp);
/* Do frame number propagation only if this is the output video node.
* Frame number either comes from the CSI receivers or it gets
--
2.1.0.rc2
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 5/7] [media] use v4l2_get_timestamp where possible
@ 2015-09-15 15:49 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-samsung-soc, Arnd Bergmann, Mauro Carvalho Chehab, y2038,
linux-api, linux-kernel
This is a preparation for a change to the type of v4l2 timestamps.
v4l2_get_timestamp() is a helper function that reads the monotonic
time and stores it into a 'struct timeval'. Multiple drivers implement
the same thing themselves for historic reasons.
Changing them all to use v4l2_get_timestamp() is more consistent
and reduces the amount of code duplication, and most importantly
simplifies the following changes.
If desired, this patch can easily be split up into one patch per
driver.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/pci/bt8xx/bttv-driver.c | 5 +----
drivers/media/pci/cx18/cx18-mailbox.c | 2 +-
drivers/media/platform/exynos4-is/fimc-lite.c | 7 +------
drivers/media/platform/omap3isp/ispstat.c | 5 ++---
drivers/media/platform/omap3isp/ispstat.h | 2 +-
drivers/media/platform/s3c-camif/camif-capture.c | 8 +-------
drivers/media/usb/gspca/gspca.c | 4 ++--
drivers/staging/media/omap4iss/iss_video.c | 5 +----
8 files changed, 10 insertions(+), 28 deletions(-)
diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
index 3632958f2158..15a4ebc2844d 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -3625,13 +3625,10 @@ static void
bttv_irq_wakeup_vbi(struct bttv *btv, struct bttv_buffer *wakeup,
unsigned int state)
{
- struct timeval ts;
-
if (NULL == wakeup)
return;
- v4l2_get_timestamp(&ts);
- wakeup->vb.ts = ts;
+ v4l2_get_timestamp(&wakeup->vb.ts);
wakeup->vb.field_count = btv->field_count;
wakeup->vb.state = state;
wake_up(&wakeup->vb.done);
diff --git a/drivers/media/pci/cx18/cx18-mailbox.c b/drivers/media/pci/cx18/cx18-mailbox.c
index eabf00c6351b..1f8aa9a749a1 100644
--- a/drivers/media/pci/cx18/cx18-mailbox.c
+++ b/drivers/media/pci/cx18/cx18-mailbox.c
@@ -202,7 +202,7 @@ static void cx18_mdl_send_to_videobuf(struct cx18_stream *s,
}
if (dispatch) {
- vb_buf->vb.ts = ktime_to_timeval(ktime_get());
+ v4l2_get_timestamp(&vb_buf->vb.ts);
list_del(&vb_buf->vb.queue);
vb_buf->vb.state = VIDEOBUF_DONE;
wake_up(&vb_buf->vb.done);
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index ca6261a86a5f..459bc65b545d 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -254,8 +254,6 @@ static irqreturn_t flite_irq_handler(int irq, void *priv)
struct fimc_lite *fimc = priv;
struct flite_buffer *vbuf;
unsigned long flags;
- struct timeval *tv;
- struct timespec ts;
u32 intsrc;
spin_lock_irqsave(&fimc->slock, flags);
@@ -294,10 +292,7 @@ static irqreturn_t flite_irq_handler(int irq, void *priv)
test_bit(ST_FLITE_RUN, &fimc->state) &&
!list_empty(&fimc->active_buf_q)) {
vbuf = fimc_lite_active_queue_pop(fimc);
- ktime_get_ts(&ts);
- tv = &vbuf->vb.v4l2_buf.timestamp;
- tv->tv_sec = ts.tv_sec;
- tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
+ v4l2_get_timestamp(&vbuf->vb.v4l2_buf.timestamp);
vbuf->vb.v4l2_buf.sequence = fimc->frame_count++;
flite_hw_mask_dma_buffer(fimc, vbuf->index);
vb2_buffer_done(&vbuf->vb, VB2_BUF_STATE_DONE);
diff --git a/drivers/media/platform/omap3isp/ispstat.c b/drivers/media/platform/omap3isp/ispstat.c
index 20434e83e801..94d4c295d3d0 100644
--- a/drivers/media/platform/omap3isp/ispstat.c
+++ b/drivers/media/platform/omap3isp/ispstat.c
@@ -235,7 +235,7 @@ static int isp_stat_buf_queue(struct ispstat *stat)
if (!stat->active_buf)
return STAT_NO_BUF;
- ktime_get_ts(&stat->active_buf->ts);
+ v4l2_get_timestamp(&stat->active_buf->ts);
stat->active_buf->buf_size = stat->buf_size;
if (isp_stat_buf_check_magic(stat, stat->active_buf)) {
@@ -496,8 +496,7 @@ int omap3isp_stat_request_statistics(struct ispstat *stat,
return PTR_ERR(buf);
}
- data->ts.tv_sec = buf->ts.tv_sec;
- data->ts.tv_usec = buf->ts.tv_nsec / NSEC_PER_USEC;
+ data->ts = buf->ts;
data->config_counter = buf->config_counter;
data->frame_number = buf->frame_number;
data->buf_size = buf->buf_size;
diff --git a/drivers/media/platform/omap3isp/ispstat.h b/drivers/media/platform/omap3isp/ispstat.h
index b79380d83fcf..6d9b0244f320 100644
--- a/drivers/media/platform/omap3isp/ispstat.h
+++ b/drivers/media/platform/omap3isp/ispstat.h
@@ -39,7 +39,7 @@ struct ispstat_buffer {
struct sg_table sgt;
void *virt_addr;
dma_addr_t dma_addr;
- struct timespec ts;
+ struct timeval ts;
u32 buf_size;
u32 frame_number;
u16 config_counter;
diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
index 76e6289a5612..edf70725ecf3 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -328,23 +328,17 @@ irqreturn_t s3c_camif_irq_handler(int irq, void *priv)
!list_empty(&vp->active_buf_q)) {
unsigned int index;
struct camif_buffer *vbuf;
- struct timeval *tv;
- struct timespec ts;
/*
* Get previous DMA write buffer index:
* 0 => DMA buffer 0, 2;
* 1 => DMA buffer 1, 3.
*/
index = (CISTATUS_FRAMECNT(status) + 2) & 1;
-
- ktime_get_ts(&ts);
vbuf = camif_active_queue_peek(vp, index);
if (!WARN_ON(vbuf == NULL)) {
/* Dequeue a filled buffer */
- tv = &vbuf->vb.v4l2_buf.timestamp;
- tv->tv_sec = ts.tv_sec;
- tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
+ v4l2_get_timestamp(&vbuf->vb.v4l2_buf.timestamp);
vbuf->vb.v4l2_buf.sequence = vp->frame_sequence++;
vb2_buffer_done(&vbuf->vb, VB2_BUF_STATE_DONE);
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index e54cee856a80..af5cd8213e8b 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -436,7 +436,7 @@ void gspca_frame_add(struct gspca_dev *gspca_dev,
}
j = gspca_dev->fr_queue[i];
frame = &gspca_dev->frame[j];
- frame->v4l2_buf.timestamp = ktime_to_timeval(ktime_get());
+ v4l2_get_timestamp(&frame->v4l2_buf.timestamp);
frame->v4l2_buf.sequence = gspca_dev->sequence++;
gspca_dev->image = frame->data;
gspca_dev->image_len = 0;
@@ -1909,7 +1909,7 @@ static ssize_t dev_read(struct file *file, char __user *data,
}
/* get a frame */
- timestamp = ktime_to_timeval(ktime_get());
+ v4l2_get_timestamp(×tamp);
timestamp.tv_sec--;
n = 2;
for (;;) {
diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index 40405d8710a6..485a90ce12df 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -420,7 +420,6 @@ struct iss_buffer *omap4iss_video_buffer_next(struct iss_video *video)
enum iss_pipeline_state state;
struct iss_buffer *buf;
unsigned long flags;
- struct timespec ts;
spin_lock_irqsave(&video->qlock, flags);
if (WARN_ON(list_empty(&video->dmaqueue))) {
@@ -433,9 +432,7 @@ struct iss_buffer *omap4iss_video_buffer_next(struct iss_video *video)
list_del(&buf->list);
spin_unlock_irqrestore(&video->qlock, flags);
- ktime_get_ts(&ts);
- buf->vb.v4l2_buf.timestamp.tv_sec = ts.tv_sec;
- buf->vb.v4l2_buf.timestamp.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
+ v4l2_get_timestamp(&buf->vb.v4l2_buf.timestamp);
/* Do frame number propagation only if this is the output video node.
* Frame number either comes from the CSI receivers or it gets
--
2.1.0.rc2
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 6/7] [RFC] [media]: v4l2: introduce v4l2_timeval
2015-09-15 15:49 ` Arnd Bergmann
@ 2015-09-15 15:49 ` Arnd Bergmann
-1 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
linux-samsung-soc, Arnd Bergmann
The v4l2 API uses a 'struct timeval' to communicate time stamps to user
space. This is broken on 32-bit architectures as soon as we have a C library
that defines time_t as 64 bit, which then changes the structure layout of
struct v4l2_buffer.
Fortunately, almost all v4l2 drivers use monotonic timestamps and call
v4l2_get_timestamp(), which means they don't also have a y2038 problem.
This means we can keep using the existing binary layout of the structure
and do not need to worry about defining a new kernel interface for
userland with 64-bit time_t.
A possible downside of this approach is that it breaks any user space
that tries to assign the timeval structure returned from the kernel
to another timeval, or to pass a pointer to it into a function that
expects a timeval. Those will cause a build-time warning or error
that can be fixed up in a backwards compatible way.
The alternative to this patch is to leave the structure using
'struct timeval', but then we have to rework the kernel to let
it handle both 32-bit and 64-bit time_t for 32-bit user space
processes.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/pci/bt8xx/bttv-driver.c | 2 +-
drivers/media/pci/meye/meye.h | 2 +-
drivers/media/pci/zoran/zoran.h | 2 +-
drivers/media/platform/coda/coda.h | 2 +-
drivers/media/platform/omap/omap_vout.c | 4 ++--
drivers/media/platform/omap3isp/ispstat.h | 2 +-
drivers/media/platform/vim2m.c | 2 +-
drivers/media/platform/vivid/vivid-ctrls.c | 2 +-
drivers/media/usb/cpia2/cpia2.h | 2 +-
drivers/media/usb/cpia2/cpia2_v4l.c | 2 +-
drivers/media/usb/gspca/gspca.c | 2 +-
drivers/media/usb/usbvision/usbvision.h | 2 +-
drivers/media/v4l2-core/v4l2-common.c | 6 +++---
include/media/v4l2-common.h | 2 +-
include/media/videobuf-core.h | 2 +-
include/trace/events/v4l2.h | 12 ++++++++++--
include/uapi/linux/omap3isp.h | 2 +-
include/uapi/linux/videodev2.h | 8 +++++++-
18 files changed, 36 insertions(+), 22 deletions(-)
diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
index 15a4ebc2844d..b0ed8d799c14 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -3585,7 +3585,7 @@ static void
bttv_irq_wakeup_video(struct bttv *btv, struct bttv_buffer_set *wakeup,
struct bttv_buffer_set *curr, unsigned int state)
{
- struct timeval ts;
+ struct v4l2_timeval ts;
v4l2_get_timestamp(&ts);
diff --git a/drivers/media/pci/meye/meye.h b/drivers/media/pci/meye/meye.h
index 751be5e533c7..a06aa5ba9abc 100644
--- a/drivers/media/pci/meye/meye.h
+++ b/drivers/media/pci/meye/meye.h
@@ -281,7 +281,7 @@
struct meye_grab_buffer {
int state; /* state of buffer */
unsigned long size; /* size of jpg frame */
- struct timeval timestamp; /* timestamp */
+ struct v4l2_timeval timestamp; /* timestamp */
unsigned long sequence; /* sequence number */
};
diff --git a/drivers/media/pci/zoran/zoran.h b/drivers/media/pci/zoran/zoran.h
index 4e7db8939c2b..9a9f782cede9 100644
--- a/drivers/media/pci/zoran/zoran.h
+++ b/drivers/media/pci/zoran/zoran.h
@@ -39,7 +39,7 @@ struct zoran_sync {
unsigned long frame; /* number of buffer that has been free'd */
unsigned long length; /* number of code bytes in buffer (capture only) */
unsigned long seq; /* frame sequence number */
- struct timeval timestamp; /* timestamp */
+ struct v4l2_timeval timestamp; /* timestamp */
};
diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
index 59b2af9c7749..fa1e15a757cd 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -138,7 +138,7 @@ struct coda_buffer_meta {
struct list_head list;
u32 sequence;
struct v4l2_timecode timecode;
- struct timeval timestamp;
+ struct v4l2_timeval timestamp;
u32 start;
u32 end;
};
diff --git a/drivers/media/platform/omap/omap_vout.c b/drivers/media/platform/omap/omap_vout.c
index 70c28d19ea04..974a238a86fe 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -513,7 +513,7 @@ static int omapvid_apply_changes(struct omap_vout_device *vout)
}
static int omapvid_handle_interlace_display(struct omap_vout_device *vout,
- unsigned int irqstatus, struct timeval timevalue)
+ unsigned int irqstatus, struct v4l2_timeval timevalue)
{
u32 fid;
@@ -557,7 +557,7 @@ static void omap_vout_isr(void *arg, unsigned int irqstatus)
int ret, fid, mgr_id;
u32 addr, irq;
struct omap_overlay *ovl;
- struct timeval timevalue;
+ struct v4l2_timeval timevalue;
struct omapvideo_info *ovid;
struct omap_dss_device *cur_display;
struct omap_vout_device *vout = (struct omap_vout_device *)arg;
diff --git a/drivers/media/platform/omap3isp/ispstat.h b/drivers/media/platform/omap3isp/ispstat.h
index 6d9b0244f320..7b4f136567a3 100644
--- a/drivers/media/platform/omap3isp/ispstat.h
+++ b/drivers/media/platform/omap3isp/ispstat.h
@@ -39,7 +39,7 @@ struct ispstat_buffer {
struct sg_table sgt;
void *virt_addr;
dma_addr_t dma_addr;
- struct timeval ts;
+ struct v4l2_timeval ts;
u32 buf_size;
u32 frame_number;
u16 config_counter;
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 295fde5fdb75..df5daac6d099 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -235,7 +235,7 @@ static int device_process(struct vim2m_ctx *ctx,
in_vb->v4l2_buf.sequence = q_data->sequence++;
memcpy(&out_vb->v4l2_buf.timestamp,
&in_vb->v4l2_buf.timestamp,
- sizeof(struct timeval));
+ sizeof(struct v4l2_timeval));
if (in_vb->v4l2_buf.flags & V4L2_BUF_FLAG_TIMECODE)
memcpy(&out_vb->v4l2_buf.timecode, &in_vb->v4l2_buf.timecode,
sizeof(struct v4l2_timecode));
diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c
index 339c8b7e53c8..065f3d6c2409 100644
--- a/drivers/media/platform/vivid/vivid-ctrls.c
+++ b/drivers/media/platform/vivid/vivid-ctrls.c
@@ -935,7 +935,7 @@ static const struct v4l2_ctrl_config vivid_ctrl_has_scaler_out = {
static int vivid_streaming_s_ctrl(struct v4l2_ctrl *ctrl)
{
struct vivid_dev *dev = container_of(ctrl->handler, struct vivid_dev, ctrl_hdl_streaming);
- struct timeval tv;
+ struct v4l2_timeval tv;
switch (ctrl->id) {
case VIVID_CID_DQBUF_ERROR:
diff --git a/drivers/media/usb/cpia2/cpia2.h b/drivers/media/usb/cpia2/cpia2.h
index cdef677d57ec..3e7c523784e7 100644
--- a/drivers/media/usb/cpia2/cpia2.h
+++ b/drivers/media/usb/cpia2/cpia2.h
@@ -354,7 +354,7 @@ struct cpia2_sbuf {
};
struct framebuf {
- struct timeval timestamp;
+ struct v4l2_timeval timestamp;
unsigned long seq;
int num;
int length;
diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c b/drivers/media/usb/cpia2/cpia2_v4l.c
index 9caea8344547..ce50d7ef4da7 100644
--- a/drivers/media/usb/cpia2/cpia2_v4l.c
+++ b/drivers/media/usb/cpia2/cpia2_v4l.c
@@ -892,7 +892,7 @@ static int find_earliest_filled_buffer(struct camera_data *cam)
found = i;
} else {
/* find which buffer is earlier */
- struct timeval *tv1, *tv2;
+ struct v4l2_timeval *tv1, *tv2;
tv1 = &cam->buffers[i].timestamp;
tv2 = &cam->buffers[found].timestamp;
if(tv1->tv_sec < tv2->tv_sec ||
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index af5cd8213e8b..c977937da9c4 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -1898,7 +1898,7 @@ static ssize_t dev_read(struct file *file, char __user *data,
struct gspca_dev *gspca_dev = video_drvdata(file);
struct gspca_frame *frame;
struct v4l2_buffer v4l2_buf;
- struct timeval timestamp;
+ struct v4l2_timeval timestamp;
int n, ret, ret2;
PDEBUG(D_FRAM, "read (%zd)", count);
diff --git a/drivers/media/usb/usbvision/usbvision.h b/drivers/media/usb/usbvision/usbvision.h
index 4f2e4fde38f2..512de31613df 100644
--- a/drivers/media/usb/usbvision/usbvision.h
+++ b/drivers/media/usb/usbvision/usbvision.h
@@ -320,7 +320,7 @@ struct usbvision_frame {
long bytes_read; /* amount of scanlength that has been read from data */
struct usbvision_v4l2_format_st v4l2_format; /* format the user needs*/
int v4l2_linesize; /* bytes for one videoline*/
- struct timeval timestamp;
+ struct v4l2_timeval timestamp;
int sequence; /* How many video frames we send to user */
};
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 5b808500e7e7..589fab615f21 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -396,11 +396,11 @@ const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(
}
EXPORT_SYMBOL_GPL(v4l2_find_nearest_format);
-void v4l2_get_timestamp(struct timeval *tv)
+void v4l2_get_timestamp(struct v4l2_timeval *tv)
{
- struct timespec ts;
+ struct timespec64 ts;
- ktime_get_ts(&ts);
+ ktime_get_ts64(&ts);
tv->tv_sec = ts.tv_sec;
tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
}
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 1cc0c5ba16b3..6e77d889c3f8 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -187,6 +187,6 @@ const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(
const struct v4l2_discrete_probe *probe,
s32 width, s32 height);
-void v4l2_get_timestamp(struct timeval *tv);
+void v4l2_get_timestamp(struct v4l2_timeval *tv);
#endif /* V4L2_COMMON_H_ */
diff --git a/include/media/videobuf-core.h b/include/media/videobuf-core.h
index d760aa73ebbb..6381338a9588 100644
--- a/include/media/videobuf-core.h
+++ b/include/media/videobuf-core.h
@@ -80,7 +80,7 @@ struct videobuf_buffer {
struct list_head queue;
wait_queue_head_t done;
unsigned int field_count;
- struct timeval ts;
+ struct v4l2_timeval ts;
/* Memory type */
enum v4l2_memory memory;
diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h
index dbf017bfddd9..a88be48dc473 100644
--- a/include/trace/events/v4l2.h
+++ b/include/trace/events/v4l2.h
@@ -6,6 +6,14 @@
#include <linux/tracepoint.h>
+#ifndef v4l2_timeval_to_ns
+#define v4l2_timeval_to_ns v4l2_timeval_to_ns
+static inline u64 v4l2_timeval_to_ns(struct v4l2_timeval *tv)
+{
+ return (u64)tv->tv_sec * NSEC_PER_SEC + tv->tv_usec * NSEC_PER_USEC;
+}
+#endif
+
/* Enums require being exported to userspace, for user tool parsing */
#undef EM
#undef EMe
@@ -126,7 +134,7 @@ DECLARE_EVENT_CLASS(v4l2_event_class,
__entry->bytesused = buf->bytesused;
__entry->flags = buf->flags;
__entry->field = buf->field;
- __entry->timestamp = timeval_to_ns(&buf->timestamp);
+ __entry->timestamp = v4l2_timeval_to_ns(&buf->timestamp);
__entry->timecode_type = buf->timecode.type;
__entry->timecode_flags = buf->timecode.flags;
__entry->timecode_frames = buf->timecode.frames;
@@ -211,7 +219,7 @@ DECLARE_EVENT_CLASS(vb2_event_class,
__entry->bytesused = vb->v4l2_planes[0].bytesused;
__entry->flags = vb->v4l2_buf.flags;
__entry->field = vb->v4l2_buf.field;
- __entry->timestamp = timeval_to_ns(&vb->v4l2_buf.timestamp);
+ __entry->timestamp = v4l2_timeval_to_ns(&vb->v4l2_buf.timestamp);
__entry->timecode_type = vb->v4l2_buf.timecode.type;
__entry->timecode_flags = vb->v4l2_buf.timecode.flags;
__entry->timecode_frames = vb->v4l2_buf.timecode.frames;
diff --git a/include/uapi/linux/omap3isp.h b/include/uapi/linux/omap3isp.h
index c090cf9249bb..2d1a085d9c9b 100644
--- a/include/uapi/linux/omap3isp.h
+++ b/include/uapi/linux/omap3isp.h
@@ -164,7 +164,7 @@ struct omap3isp_h3a_aewb_config {
* @config_counter: Number of the configuration associated with the data.
*/
struct omap3isp_stat_data {
- struct timeval ts;
+ struct v4l2_timeval ts;
void __user *buf;
__u32 buf_size;
__u16 frame_number;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 3228fbebcd63..b02cf054fbb8 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -803,6 +803,12 @@ struct v4l2_plane {
__u32 reserved[11];
};
+/* used for monotonic times, therefore y2038 safe */
+struct v4l2_timeval {
+ long tv_sec;
+ long tv_usec;
+};
+
/**
* struct v4l2_buffer - video buffer info
* @index: id number of the buffer
@@ -839,7 +845,7 @@ struct v4l2_buffer {
__u32 bytesused;
__u32 flags;
__u32 field;
- struct timeval timestamp;
+ struct v4l2_timeval timestamp;
struct v4l2_timecode timecode;
__u32 sequence;
--
2.1.0.rc2
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 6/7] [RFC] [media]: v4l2: introduce v4l2_timeval
@ 2015-09-15 15:49 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-samsung-soc, Arnd Bergmann, Mauro Carvalho Chehab, y2038,
linux-api, linux-kernel
The v4l2 API uses a 'struct timeval' to communicate time stamps to user
space. This is broken on 32-bit architectures as soon as we have a C library
that defines time_t as 64 bit, which then changes the structure layout of
struct v4l2_buffer.
Fortunately, almost all v4l2 drivers use monotonic timestamps and call
v4l2_get_timestamp(), which means they don't also have a y2038 problem.
This means we can keep using the existing binary layout of the structure
and do not need to worry about defining a new kernel interface for
userland with 64-bit time_t.
A possible downside of this approach is that it breaks any user space
that tries to assign the timeval structure returned from the kernel
to another timeval, or to pass a pointer to it into a function that
expects a timeval. Those will cause a build-time warning or error
that can be fixed up in a backwards compatible way.
The alternative to this patch is to leave the structure using
'struct timeval', but then we have to rework the kernel to let
it handle both 32-bit and 64-bit time_t for 32-bit user space
processes.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/pci/bt8xx/bttv-driver.c | 2 +-
drivers/media/pci/meye/meye.h | 2 +-
drivers/media/pci/zoran/zoran.h | 2 +-
drivers/media/platform/coda/coda.h | 2 +-
drivers/media/platform/omap/omap_vout.c | 4 ++--
drivers/media/platform/omap3isp/ispstat.h | 2 +-
drivers/media/platform/vim2m.c | 2 +-
drivers/media/platform/vivid/vivid-ctrls.c | 2 +-
drivers/media/usb/cpia2/cpia2.h | 2 +-
drivers/media/usb/cpia2/cpia2_v4l.c | 2 +-
drivers/media/usb/gspca/gspca.c | 2 +-
drivers/media/usb/usbvision/usbvision.h | 2 +-
drivers/media/v4l2-core/v4l2-common.c | 6 +++---
include/media/v4l2-common.h | 2 +-
include/media/videobuf-core.h | 2 +-
include/trace/events/v4l2.h | 12 ++++++++++--
include/uapi/linux/omap3isp.h | 2 +-
include/uapi/linux/videodev2.h | 8 +++++++-
18 files changed, 36 insertions(+), 22 deletions(-)
diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
index 15a4ebc2844d..b0ed8d799c14 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -3585,7 +3585,7 @@ static void
bttv_irq_wakeup_video(struct bttv *btv, struct bttv_buffer_set *wakeup,
struct bttv_buffer_set *curr, unsigned int state)
{
- struct timeval ts;
+ struct v4l2_timeval ts;
v4l2_get_timestamp(&ts);
diff --git a/drivers/media/pci/meye/meye.h b/drivers/media/pci/meye/meye.h
index 751be5e533c7..a06aa5ba9abc 100644
--- a/drivers/media/pci/meye/meye.h
+++ b/drivers/media/pci/meye/meye.h
@@ -281,7 +281,7 @@
struct meye_grab_buffer {
int state; /* state of buffer */
unsigned long size; /* size of jpg frame */
- struct timeval timestamp; /* timestamp */
+ struct v4l2_timeval timestamp; /* timestamp */
unsigned long sequence; /* sequence number */
};
diff --git a/drivers/media/pci/zoran/zoran.h b/drivers/media/pci/zoran/zoran.h
index 4e7db8939c2b..9a9f782cede9 100644
--- a/drivers/media/pci/zoran/zoran.h
+++ b/drivers/media/pci/zoran/zoran.h
@@ -39,7 +39,7 @@ struct zoran_sync {
unsigned long frame; /* number of buffer that has been free'd */
unsigned long length; /* number of code bytes in buffer (capture only) */
unsigned long seq; /* frame sequence number */
- struct timeval timestamp; /* timestamp */
+ struct v4l2_timeval timestamp; /* timestamp */
};
diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
index 59b2af9c7749..fa1e15a757cd 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -138,7 +138,7 @@ struct coda_buffer_meta {
struct list_head list;
u32 sequence;
struct v4l2_timecode timecode;
- struct timeval timestamp;
+ struct v4l2_timeval timestamp;
u32 start;
u32 end;
};
diff --git a/drivers/media/platform/omap/omap_vout.c b/drivers/media/platform/omap/omap_vout.c
index 70c28d19ea04..974a238a86fe 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -513,7 +513,7 @@ static int omapvid_apply_changes(struct omap_vout_device *vout)
}
static int omapvid_handle_interlace_display(struct omap_vout_device *vout,
- unsigned int irqstatus, struct timeval timevalue)
+ unsigned int irqstatus, struct v4l2_timeval timevalue)
{
u32 fid;
@@ -557,7 +557,7 @@ static void omap_vout_isr(void *arg, unsigned int irqstatus)
int ret, fid, mgr_id;
u32 addr, irq;
struct omap_overlay *ovl;
- struct timeval timevalue;
+ struct v4l2_timeval timevalue;
struct omapvideo_info *ovid;
struct omap_dss_device *cur_display;
struct omap_vout_device *vout = (struct omap_vout_device *)arg;
diff --git a/drivers/media/platform/omap3isp/ispstat.h b/drivers/media/platform/omap3isp/ispstat.h
index 6d9b0244f320..7b4f136567a3 100644
--- a/drivers/media/platform/omap3isp/ispstat.h
+++ b/drivers/media/platform/omap3isp/ispstat.h
@@ -39,7 +39,7 @@ struct ispstat_buffer {
struct sg_table sgt;
void *virt_addr;
dma_addr_t dma_addr;
- struct timeval ts;
+ struct v4l2_timeval ts;
u32 buf_size;
u32 frame_number;
u16 config_counter;
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 295fde5fdb75..df5daac6d099 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -235,7 +235,7 @@ static int device_process(struct vim2m_ctx *ctx,
in_vb->v4l2_buf.sequence = q_data->sequence++;
memcpy(&out_vb->v4l2_buf.timestamp,
&in_vb->v4l2_buf.timestamp,
- sizeof(struct timeval));
+ sizeof(struct v4l2_timeval));
if (in_vb->v4l2_buf.flags & V4L2_BUF_FLAG_TIMECODE)
memcpy(&out_vb->v4l2_buf.timecode, &in_vb->v4l2_buf.timecode,
sizeof(struct v4l2_timecode));
diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c
index 339c8b7e53c8..065f3d6c2409 100644
--- a/drivers/media/platform/vivid/vivid-ctrls.c
+++ b/drivers/media/platform/vivid/vivid-ctrls.c
@@ -935,7 +935,7 @@ static const struct v4l2_ctrl_config vivid_ctrl_has_scaler_out = {
static int vivid_streaming_s_ctrl(struct v4l2_ctrl *ctrl)
{
struct vivid_dev *dev = container_of(ctrl->handler, struct vivid_dev, ctrl_hdl_streaming);
- struct timeval tv;
+ struct v4l2_timeval tv;
switch (ctrl->id) {
case VIVID_CID_DQBUF_ERROR:
diff --git a/drivers/media/usb/cpia2/cpia2.h b/drivers/media/usb/cpia2/cpia2.h
index cdef677d57ec..3e7c523784e7 100644
--- a/drivers/media/usb/cpia2/cpia2.h
+++ b/drivers/media/usb/cpia2/cpia2.h
@@ -354,7 +354,7 @@ struct cpia2_sbuf {
};
struct framebuf {
- struct timeval timestamp;
+ struct v4l2_timeval timestamp;
unsigned long seq;
int num;
int length;
diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c b/drivers/media/usb/cpia2/cpia2_v4l.c
index 9caea8344547..ce50d7ef4da7 100644
--- a/drivers/media/usb/cpia2/cpia2_v4l.c
+++ b/drivers/media/usb/cpia2/cpia2_v4l.c
@@ -892,7 +892,7 @@ static int find_earliest_filled_buffer(struct camera_data *cam)
found = i;
} else {
/* find which buffer is earlier */
- struct timeval *tv1, *tv2;
+ struct v4l2_timeval *tv1, *tv2;
tv1 = &cam->buffers[i].timestamp;
tv2 = &cam->buffers[found].timestamp;
if(tv1->tv_sec < tv2->tv_sec ||
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index af5cd8213e8b..c977937da9c4 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -1898,7 +1898,7 @@ static ssize_t dev_read(struct file *file, char __user *data,
struct gspca_dev *gspca_dev = video_drvdata(file);
struct gspca_frame *frame;
struct v4l2_buffer v4l2_buf;
- struct timeval timestamp;
+ struct v4l2_timeval timestamp;
int n, ret, ret2;
PDEBUG(D_FRAM, "read (%zd)", count);
diff --git a/drivers/media/usb/usbvision/usbvision.h b/drivers/media/usb/usbvision/usbvision.h
index 4f2e4fde38f2..512de31613df 100644
--- a/drivers/media/usb/usbvision/usbvision.h
+++ b/drivers/media/usb/usbvision/usbvision.h
@@ -320,7 +320,7 @@ struct usbvision_frame {
long bytes_read; /* amount of scanlength that has been read from data */
struct usbvision_v4l2_format_st v4l2_format; /* format the user needs*/
int v4l2_linesize; /* bytes for one videoline*/
- struct timeval timestamp;
+ struct v4l2_timeval timestamp;
int sequence; /* How many video frames we send to user */
};
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 5b808500e7e7..589fab615f21 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -396,11 +396,11 @@ const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(
}
EXPORT_SYMBOL_GPL(v4l2_find_nearest_format);
-void v4l2_get_timestamp(struct timeval *tv)
+void v4l2_get_timestamp(struct v4l2_timeval *tv)
{
- struct timespec ts;
+ struct timespec64 ts;
- ktime_get_ts(&ts);
+ ktime_get_ts64(&ts);
tv->tv_sec = ts.tv_sec;
tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
}
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 1cc0c5ba16b3..6e77d889c3f8 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -187,6 +187,6 @@ const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(
const struct v4l2_discrete_probe *probe,
s32 width, s32 height);
-void v4l2_get_timestamp(struct timeval *tv);
+void v4l2_get_timestamp(struct v4l2_timeval *tv);
#endif /* V4L2_COMMON_H_ */
diff --git a/include/media/videobuf-core.h b/include/media/videobuf-core.h
index d760aa73ebbb..6381338a9588 100644
--- a/include/media/videobuf-core.h
+++ b/include/media/videobuf-core.h
@@ -80,7 +80,7 @@ struct videobuf_buffer {
struct list_head queue;
wait_queue_head_t done;
unsigned int field_count;
- struct timeval ts;
+ struct v4l2_timeval ts;
/* Memory type */
enum v4l2_memory memory;
diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h
index dbf017bfddd9..a88be48dc473 100644
--- a/include/trace/events/v4l2.h
+++ b/include/trace/events/v4l2.h
@@ -6,6 +6,14 @@
#include <linux/tracepoint.h>
+#ifndef v4l2_timeval_to_ns
+#define v4l2_timeval_to_ns v4l2_timeval_to_ns
+static inline u64 v4l2_timeval_to_ns(struct v4l2_timeval *tv)
+{
+ return (u64)tv->tv_sec * NSEC_PER_SEC + tv->tv_usec * NSEC_PER_USEC;
+}
+#endif
+
/* Enums require being exported to userspace, for user tool parsing */
#undef EM
#undef EMe
@@ -126,7 +134,7 @@ DECLARE_EVENT_CLASS(v4l2_event_class,
__entry->bytesused = buf->bytesused;
__entry->flags = buf->flags;
__entry->field = buf->field;
- __entry->timestamp = timeval_to_ns(&buf->timestamp);
+ __entry->timestamp = v4l2_timeval_to_ns(&buf->timestamp);
__entry->timecode_type = buf->timecode.type;
__entry->timecode_flags = buf->timecode.flags;
__entry->timecode_frames = buf->timecode.frames;
@@ -211,7 +219,7 @@ DECLARE_EVENT_CLASS(vb2_event_class,
__entry->bytesused = vb->v4l2_planes[0].bytesused;
__entry->flags = vb->v4l2_buf.flags;
__entry->field = vb->v4l2_buf.field;
- __entry->timestamp = timeval_to_ns(&vb->v4l2_buf.timestamp);
+ __entry->timestamp = v4l2_timeval_to_ns(&vb->v4l2_buf.timestamp);
__entry->timecode_type = vb->v4l2_buf.timecode.type;
__entry->timecode_flags = vb->v4l2_buf.timecode.flags;
__entry->timecode_frames = vb->v4l2_buf.timecode.frames;
diff --git a/include/uapi/linux/omap3isp.h b/include/uapi/linux/omap3isp.h
index c090cf9249bb..2d1a085d9c9b 100644
--- a/include/uapi/linux/omap3isp.h
+++ b/include/uapi/linux/omap3isp.h
@@ -164,7 +164,7 @@ struct omap3isp_h3a_aewb_config {
* @config_counter: Number of the configuration associated with the data.
*/
struct omap3isp_stat_data {
- struct timeval ts;
+ struct v4l2_timeval ts;
void __user *buf;
__u32 buf_size;
__u16 frame_number;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 3228fbebcd63..b02cf054fbb8 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -803,6 +803,12 @@ struct v4l2_plane {
__u32 reserved[11];
};
+/* used for monotonic times, therefore y2038 safe */
+struct v4l2_timeval {
+ long tv_sec;
+ long tv_usec;
+};
+
/**
* struct v4l2_buffer - video buffer info
* @index: id number of the buffer
@@ -839,7 +845,7 @@ struct v4l2_buffer {
__u32 bytesused;
__u32 flags;
__u32 field;
- struct timeval timestamp;
+ struct v4l2_timeval timestamp;
struct v4l2_timecode timecode;
__u32 sequence;
--
2.1.0.rc2
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 7/7] [RFC] [media] introduce v4l2_timespec type for timestamps
2015-09-15 15:49 ` Arnd Bergmann
@ 2015-09-15 15:49 ` Arnd Bergmann
-1 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
linux-samsung-soc, Arnd Bergmann
The v4l2 event queue uses a 'struct timespec' to pass monotonic
timestamps. This is not a problem by itself, but it breaks when user
space redefines timespec to use 'long long' on 32-bit systems.
To avoid that problem, we define our own replacement for timespec here,
using 'long' tv_sec and tv_nsec members. This means no change to the
existing API, but lets us keep using it with a y2038 compatible libc.
As a side-effect, this changes the API for x32 programs to be the same
as native 32-bit, using a 32-bit tv_sec/tv_nsec value, but both old and
new kernels already support both formats for on the binary level for
compat and x32.
Alternatively, we could leave the header file to define the interface
based on 'struct timespec', and implement both ioctls for native
processes. I picked the approach in this case because it matches what
we do for v4l2_timeval in the respective patch, but both would work
equally well here.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/v4l2-core/v4l2-event.c | 20 +++++++++++++-------
include/uapi/linux/videodev2.h | 8 +++++++-
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-event.c b/drivers/media/v4l2-core/v4l2-event.c
index 8d3171c6bee8..2a26ad591f59 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -108,7 +108,7 @@ static struct v4l2_subscribed_event *v4l2_event_subscribed(
}
static void __v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev,
- const struct timespec *ts)
+ const struct v4l2_timespec *ts)
{
struct v4l2_subscribed_event *sev;
struct v4l2_kevent *kev;
@@ -170,17 +170,20 @@ void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event *ev)
{
struct v4l2_fh *fh;
unsigned long flags;
- struct timespec timestamp;
+ struct timespec64 timestamp;
+ struct v4l2_timespec vts;
if (vdev == NULL)
return;
- ktime_get_ts(×tamp);
+ ktime_get_ts64(×tamp);
+ vts.tv_sec = timestamp.tv_sec;
+ vts.tv_nsec = timestamp.tv_nsec;
spin_lock_irqsave(&vdev->fh_lock, flags);
list_for_each_entry(fh, &vdev->fh_list, list)
- __v4l2_event_queue_fh(fh, ev, ×tamp);
+ __v4l2_event_queue_fh(fh, ev, &vts);
spin_unlock_irqrestore(&vdev->fh_lock, flags);
}
@@ -189,12 +192,15 @@ EXPORT_SYMBOL_GPL(v4l2_event_queue);
void v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev)
{
unsigned long flags;
- struct timespec timestamp;
+ struct timespec64 timestamp;
+ struct v4l2_timespec vts;
- ktime_get_ts(×tamp);
+ ktime_get_ts64(×tamp);
+ vts.tv_sec = timestamp.tv_sec;
+ vts.tv_nsec = timestamp.tv_nsec;
spin_lock_irqsave(&fh->vdev->fh_lock, flags);
- __v4l2_event_queue_fh(fh, ev, ×tamp);
+ __v4l2_event_queue_fh(fh, ev, &vts);
spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
}
EXPORT_SYMBOL_GPL(v4l2_event_queue_fh);
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index b02cf054fbb8..bc659be87260 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -809,6 +809,12 @@ struct v4l2_timeval {
long tv_usec;
};
+/* used for monotonic times, therefore y2038 safe */
+struct v4l2_timespec {
+ long tv_sec;
+ long tv_nsec;
+};
+
/**
* struct v4l2_buffer - video buffer info
* @index: id number of the buffer
@@ -2088,7 +2094,7 @@ struct v4l2_event {
} u;
__u32 pending;
__u32 sequence;
- struct timespec timestamp;
+ struct v4l2_timespec timestamp;
__u32 id;
__u32 reserved[8];
};
--
2.1.0.rc2
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 7/7] [RFC] [media] introduce v4l2_timespec type for timestamps
@ 2015-09-15 15:49 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 15:49 UTC (permalink / raw)
To: linux-media
Cc: linux-samsung-soc, Arnd Bergmann, Mauro Carvalho Chehab, y2038,
linux-api, linux-kernel
The v4l2 event queue uses a 'struct timespec' to pass monotonic
timestamps. This is not a problem by itself, but it breaks when user
space redefines timespec to use 'long long' on 32-bit systems.
To avoid that problem, we define our own replacement for timespec here,
using 'long' tv_sec and tv_nsec members. This means no change to the
existing API, but lets us keep using it with a y2038 compatible libc.
As a side-effect, this changes the API for x32 programs to be the same
as native 32-bit, using a 32-bit tv_sec/tv_nsec value, but both old and
new kernels already support both formats for on the binary level for
compat and x32.
Alternatively, we could leave the header file to define the interface
based on 'struct timespec', and implement both ioctls for native
processes. I picked the approach in this case because it matches what
we do for v4l2_timeval in the respective patch, but both would work
equally well here.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/v4l2-core/v4l2-event.c | 20 +++++++++++++-------
include/uapi/linux/videodev2.h | 8 +++++++-
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-event.c b/drivers/media/v4l2-core/v4l2-event.c
index 8d3171c6bee8..2a26ad591f59 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -108,7 +108,7 @@ static struct v4l2_subscribed_event *v4l2_event_subscribed(
}
static void __v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev,
- const struct timespec *ts)
+ const struct v4l2_timespec *ts)
{
struct v4l2_subscribed_event *sev;
struct v4l2_kevent *kev;
@@ -170,17 +170,20 @@ void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event *ev)
{
struct v4l2_fh *fh;
unsigned long flags;
- struct timespec timestamp;
+ struct timespec64 timestamp;
+ struct v4l2_timespec vts;
if (vdev == NULL)
return;
- ktime_get_ts(×tamp);
+ ktime_get_ts64(×tamp);
+ vts.tv_sec = timestamp.tv_sec;
+ vts.tv_nsec = timestamp.tv_nsec;
spin_lock_irqsave(&vdev->fh_lock, flags);
list_for_each_entry(fh, &vdev->fh_list, list)
- __v4l2_event_queue_fh(fh, ev, ×tamp);
+ __v4l2_event_queue_fh(fh, ev, &vts);
spin_unlock_irqrestore(&vdev->fh_lock, flags);
}
@@ -189,12 +192,15 @@ EXPORT_SYMBOL_GPL(v4l2_event_queue);
void v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev)
{
unsigned long flags;
- struct timespec timestamp;
+ struct timespec64 timestamp;
+ struct v4l2_timespec vts;
- ktime_get_ts(×tamp);
+ ktime_get_ts64(×tamp);
+ vts.tv_sec = timestamp.tv_sec;
+ vts.tv_nsec = timestamp.tv_nsec;
spin_lock_irqsave(&fh->vdev->fh_lock, flags);
- __v4l2_event_queue_fh(fh, ev, ×tamp);
+ __v4l2_event_queue_fh(fh, ev, &vts);
spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
}
EXPORT_SYMBOL_GPL(v4l2_event_queue_fh);
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index b02cf054fbb8..bc659be87260 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -809,6 +809,12 @@ struct v4l2_timeval {
long tv_usec;
};
+/* used for monotonic times, therefore y2038 safe */
+struct v4l2_timespec {
+ long tv_sec;
+ long tv_nsec;
+};
+
/**
* struct v4l2_buffer - video buffer info
* @index: id number of the buffer
@@ -2088,7 +2094,7 @@ struct v4l2_event {
} u;
__u32 pending;
__u32 sequence;
- struct timespec timestamp;
+ struct v4l2_timespec timestamp;
__u32 id;
__u32 reserved[8];
};
--
2.1.0.rc2
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH 6/7] [RFC] [media]: v4l2: introduce v4l2_timeval
@ 2015-09-15 16:27 ` Hans Verkuil
0 siblings, 0 replies; 42+ messages in thread
From: Hans Verkuil @ 2015-09-15 16:27 UTC (permalink / raw)
To: Arnd Bergmann, linux-media
Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api, linux-samsung-soc
On 09/15/2015 05:49 PM, Arnd Bergmann wrote:
> The v4l2 API uses a 'struct timeval' to communicate time stamps to user
> space. This is broken on 32-bit architectures as soon as we have a C library
> that defines time_t as 64 bit, which then changes the structure layout of
> struct v4l2_buffer.
>
> Fortunately, almost all v4l2 drivers use monotonic timestamps and call
> v4l2_get_timestamp(), which means they don't also have a y2038 problem.
> This means we can keep using the existing binary layout of the structure
> and do not need to worry about defining a new kernel interface for
> userland with 64-bit time_t.
>
> A possible downside of this approach is that it breaks any user space
> that tries to assign the timeval structure returned from the kernel
> to another timeval, or to pass a pointer to it into a function that
> expects a timeval. Those will cause a build-time warning or error
> that can be fixed up in a backwards compatible way.
>
> The alternative to this patch is to leave the structure using
> 'struct timeval', but then we have to rework the kernel to let
> it handle both 32-bit and 64-bit time_t for 32-bit user space
> processes.
Cool. Only this morning I was thinking about what would be needed in v4l2
to be y2038 safe, and here it is!
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 3228fbebcd63..b02cf054fbb8 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -803,6 +803,12 @@ struct v4l2_plane {
> __u32 reserved[11];
> };
>
> +/* used for monotonic times, therefore y2038 safe */
> +struct v4l2_timeval {
> + long tv_sec;
> + long tv_usec;
> +};
> +
> /**
> * struct v4l2_buffer - video buffer info
> * @index: id number of the buffer
> @@ -839,7 +845,7 @@ struct v4l2_buffer {
> __u32 bytesused;
> __u32 flags;
> __u32 field;
> - struct timeval timestamp;
> + struct v4l2_timeval timestamp;
> struct v4l2_timecode timecode;
> __u32 sequence;
>
>
I suspect that quite a few apps use assign the timestamp to another timeval
struct. A quick grep in v4l-utils (which we maintain) shows at least two of
those assignments. Ditto for xawtv3.
So I don't think v4l2_timeval is an option as it would break userspace too badly.
An alternative to supporting a 64-bit timeval for 32-bit userspace is to make a
new y2038-aware struct and a new set of ioctls and use this opportunity to clean
up and extend the v4l2_buffer struct.
So any 32-bit app that needs to be y2038 compliant would just use the new
struct and ioctls.
But this is something to discuss among the v4l2 developers.
Regards,
Hans
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 6/7] [RFC] [media]: v4l2: introduce v4l2_timeval
@ 2015-09-15 16:27 ` Hans Verkuil
0 siblings, 0 replies; 42+ messages in thread
From: Hans Verkuil @ 2015-09-15 16:27 UTC (permalink / raw)
To: Arnd Bergmann, linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
y2038-cunTk1MwBs8s++Sfvej+rw, Mauro Carvalho Chehab,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA
On 09/15/2015 05:49 PM, Arnd Bergmann wrote:
> The v4l2 API uses a 'struct timeval' to communicate time stamps to user
> space. This is broken on 32-bit architectures as soon as we have a C library
> that defines time_t as 64 bit, which then changes the structure layout of
> struct v4l2_buffer.
>
> Fortunately, almost all v4l2 drivers use monotonic timestamps and call
> v4l2_get_timestamp(), which means they don't also have a y2038 problem.
> This means we can keep using the existing binary layout of the structure
> and do not need to worry about defining a new kernel interface for
> userland with 64-bit time_t.
>
> A possible downside of this approach is that it breaks any user space
> that tries to assign the timeval structure returned from the kernel
> to another timeval, or to pass a pointer to it into a function that
> expects a timeval. Those will cause a build-time warning or error
> that can be fixed up in a backwards compatible way.
>
> The alternative to this patch is to leave the structure using
> 'struct timeval', but then we have to rework the kernel to let
> it handle both 32-bit and 64-bit time_t for 32-bit user space
> processes.
Cool. Only this morning I was thinking about what would be needed in v4l2
to be y2038 safe, and here it is!
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 3228fbebcd63..b02cf054fbb8 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -803,6 +803,12 @@ struct v4l2_plane {
> __u32 reserved[11];
> };
>
> +/* used for monotonic times, therefore y2038 safe */
> +struct v4l2_timeval {
> + long tv_sec;
> + long tv_usec;
> +};
> +
> /**
> * struct v4l2_buffer - video buffer info
> * @index: id number of the buffer
> @@ -839,7 +845,7 @@ struct v4l2_buffer {
> __u32 bytesused;
> __u32 flags;
> __u32 field;
> - struct timeval timestamp;
> + struct v4l2_timeval timestamp;
> struct v4l2_timecode timecode;
> __u32 sequence;
>
>
I suspect that quite a few apps use assign the timestamp to another timeval
struct. A quick grep in v4l-utils (which we maintain) shows at least two of
those assignments. Ditto for xawtv3.
So I don't think v4l2_timeval is an option as it would break userspace too badly.
An alternative to supporting a 64-bit timeval for 32-bit userspace is to make a
new y2038-aware struct and a new set of ioctls and use this opportunity to clean
up and extend the v4l2_buffer struct.
So any 32-bit app that needs to be y2038 compliant would just use the new
struct and ioctls.
But this is something to discuss among the v4l2 developers.
Regards,
Hans
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 7/7] [RFC] [media] introduce v4l2_timespec type for timestamps
2015-09-15 15:49 ` Arnd Bergmann
(?)
@ 2015-09-15 16:32 ` Hans Verkuil
2015-09-15 20:27 ` Arnd Bergmann
-1 siblings, 1 reply; 42+ messages in thread
From: Hans Verkuil @ 2015-09-15 16:32 UTC (permalink / raw)
To: Arnd Bergmann, linux-media
Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api, linux-samsung-soc
On 09/15/2015 05:49 PM, Arnd Bergmann wrote:
> The v4l2 event queue uses a 'struct timespec' to pass monotonic
> timestamps. This is not a problem by itself, but it breaks when user
> space redefines timespec to use 'long long' on 32-bit systems.
>
> To avoid that problem, we define our own replacement for timespec here,
> using 'long' tv_sec and tv_nsec members. This means no change to the
> existing API, but lets us keep using it with a y2038 compatible libc.
>
> As a side-effect, this changes the API for x32 programs to be the same
> as native 32-bit, using a 32-bit tv_sec/tv_nsec value, but both old and
> new kernels already support both formats for on the binary level for
> compat and x32.
>
> Alternatively, we could leave the header file to define the interface
> based on 'struct timespec', and implement both ioctls for native
> processes. I picked the approach in this case because it matches what
> we do for v4l2_timeval in the respective patch, but both would work
> equally well here.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/media/v4l2-core/v4l2-event.c | 20 +++++++++++++-------
> include/uapi/linux/videodev2.h | 8 +++++++-
> 2 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-event.c b/drivers/media/v4l2-core/v4l2-event.c
> index 8d3171c6bee8..2a26ad591f59 100644
> --- a/drivers/media/v4l2-core/v4l2-event.c
> +++ b/drivers/media/v4l2-core/v4l2-event.c
> @@ -108,7 +108,7 @@ static struct v4l2_subscribed_event *v4l2_event_subscribed(
> }
>
> static void __v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev,
> - const struct timespec *ts)
> + const struct v4l2_timespec *ts)
> {
> struct v4l2_subscribed_event *sev;
> struct v4l2_kevent *kev;
> @@ -170,17 +170,20 @@ void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event *ev)
> {
> struct v4l2_fh *fh;
> unsigned long flags;
> - struct timespec timestamp;
> + struct timespec64 timestamp;
> + struct v4l2_timespec vts;
>
> if (vdev == NULL)
> return;
>
> - ktime_get_ts(×tamp);
> + ktime_get_ts64(×tamp);
> + vts.tv_sec = timestamp.tv_sec;
> + vts.tv_nsec = timestamp.tv_nsec;
I prefer to take this opportunity to create a v4l2_get_timespec helper
function, just like v4l2_get_timeval.
>
> spin_lock_irqsave(&vdev->fh_lock, flags);
>
> list_for_each_entry(fh, &vdev->fh_list, list)
> - __v4l2_event_queue_fh(fh, ev, ×tamp);
> + __v4l2_event_queue_fh(fh, ev, &vts);
>
> spin_unlock_irqrestore(&vdev->fh_lock, flags);
> }
> @@ -189,12 +192,15 @@ EXPORT_SYMBOL_GPL(v4l2_event_queue);
> void v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev)
> {
> unsigned long flags;
> - struct timespec timestamp;
> + struct timespec64 timestamp;
> + struct v4l2_timespec vts;
>
> - ktime_get_ts(×tamp);
> + ktime_get_ts64(×tamp);
> + vts.tv_sec = timestamp.tv_sec;
> + vts.tv_nsec = timestamp.tv_nsec;
>
> spin_lock_irqsave(&fh->vdev->fh_lock, flags);
> - __v4l2_event_queue_fh(fh, ev, ×tamp);
> + __v4l2_event_queue_fh(fh, ev, &vts);
> spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
> }
> EXPORT_SYMBOL_GPL(v4l2_event_queue_fh);
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index b02cf054fbb8..bc659be87260 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -809,6 +809,12 @@ struct v4l2_timeval {
> long tv_usec;
> };
>
> +/* used for monotonic times, therefore y2038 safe */
> +struct v4l2_timespec {
> + long tv_sec;
> + long tv_nsec;
> +};
> +
> /**
> * struct v4l2_buffer - video buffer info
> * @index: id number of the buffer
> @@ -2088,7 +2094,7 @@ struct v4l2_event {
> } u;
> __u32 pending;
> __u32 sequence;
> - struct timespec timestamp;
> + struct v4l2_timespec timestamp;
> __u32 id;
> __u32 reserved[8];
> };
>
I think I am OK with this. This timestamp is used much more rarely and I do
not expect this ABI change to cause any problems in userspace.
Regards,
Hans
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/7] [media] dvb: use ktime_t for internal timeout
2015-09-15 15:49 ` Arnd Bergmann
@ 2015-09-15 17:55 ` Andreas Oberritter
-1 siblings, 0 replies; 42+ messages in thread
From: Andreas Oberritter @ 2015-09-15 17:55 UTC (permalink / raw)
To: Arnd Bergmann, linux-media
Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api, linux-samsung-soc
Hello Arnd,
On 15.09.2015 17:49, Arnd Bergmann wrote:
> The dvb demuxer code uses a 'struct timespec' to pass a timeout
> as absolute time. This will cause problems on 32-bit architectures
> in 2038 when time_t overflows, and it is racy with a concurrent
> settimeofday() call.
>
> This patch changes the code to use ktime_get() instead, using
> the monotonic time base to avoid both the race and the overflow.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/media/dvb-core/demux.h | 2 +-
> drivers/media/dvb-core/dmxdev.c | 2 +-
> drivers/media/dvb-core/dvb_demux.c | 17 ++++++-----------
> drivers/media/dvb-core/dvb_demux.h | 4 ++--
> drivers/media/dvb-core/dvb_net.c | 2 +-
> 5 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/dvb-core/demux.h b/drivers/media/dvb-core/demux.h
> index 833191bcd810..d8e2b1213bef 100644
> --- a/drivers/media/dvb-core/demux.h
> +++ b/drivers/media/dvb-core/demux.h
> @@ -92,7 +92,7 @@ struct dmx_ts_feed {
> int type,
> enum dmx_ts_pes pes_type,
> size_t circular_buffer_size,
> - struct timespec timeout);
> + ktime_t timeout);
> int (*start_filtering) (struct dmx_ts_feed* feed);
> int (*stop_filtering) (struct dmx_ts_feed* feed);
> };
> diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
> index d0e3f9d85f34..0d20b379eeec 100644
> --- a/drivers/media/dvb-core/dmxdev.c
> +++ b/drivers/media/dvb-core/dmxdev.c
> @@ -558,7 +558,7 @@ static int dvb_dmxdev_start_feed(struct dmxdev *dmxdev,
> struct dmxdev_filter *filter,
> struct dmxdev_feed *feed)
> {
> - struct timespec timeout = { 0 };
> + ktime_t timeout = ktime_set(0, 0);
> struct dmx_pes_filter_params *para = &filter->params.pes;
> dmx_output_t otype;
> int ret;
> diff --git a/drivers/media/dvb-core/dvb_demux.c b/drivers/media/dvb-core/dvb_demux.c
> index 6c7ff0cdcd32..d83dd0eb5757 100644
> --- a/drivers/media/dvb-core/dvb_demux.c
> +++ b/drivers/media/dvb-core/dvb_demux.c
> @@ -399,28 +399,23 @@ static void dvb_dmx_swfilter_packet(struct dvb_demux *demux, const u8 *buf)
> int dvr_done = 0;
>
> if (dvb_demux_speedcheck) {
> - struct timespec cur_time, delta_time;
> + ktime_t cur_time;
> u64 speed_bytes, speed_timedelta;
>
> demux->speed_pkts_cnt++;
>
> /* show speed every SPEED_PKTS_INTERVAL packets */
> if (!(demux->speed_pkts_cnt % SPEED_PKTS_INTERVAL)) {
> - cur_time = current_kernel_time();
> + cur_time = ktime_get();
>
> - if (demux->speed_last_time.tv_sec != 0 &&
> - demux->speed_last_time.tv_nsec != 0) {
> - delta_time = timespec_sub(cur_time,
> - demux->speed_last_time);
> + if (ktime_to_ns(demux->speed_last_time) == 0) {
if ktime_to_ns does what I think it does, then you should invert the logic.
Regards,
Andreas
> speed_bytes = (u64)demux->speed_pkts_cnt
> * 188 * 8;
> /* convert to 1024 basis */
> speed_bytes = 1000 * div64_u64(speed_bytes,
> 1024);
> - speed_timedelta =
> - (u64)timespec_to_ns(&delta_time);
> - speed_timedelta = div64_u64(speed_timedelta,
> - 1000000); /* nsec -> usec */
> + speed_timedelta = ktime_ms_delta(cur_time,
> + demux->speed_last_time);
> printk(KERN_INFO "TS speed %llu Kbits/sec \n",
> div64_u64(speed_bytes,
> speed_timedelta));
> @@ -667,7 +662,7 @@ out:
>
> static int dmx_ts_feed_set(struct dmx_ts_feed *ts_feed, u16 pid, int ts_type,
> enum dmx_ts_pes pes_type,
> - size_t circular_buffer_size, struct timespec timeout)
> + size_t circular_buffer_size, ktime_t timeout)
> {
> struct dvb_demux_feed *feed = (struct dvb_demux_feed *)ts_feed;
> struct dvb_demux *demux = feed->demux;
> diff --git a/drivers/media/dvb-core/dvb_demux.h b/drivers/media/dvb-core/dvb_demux.h
> index ae7fc33c3231..5ed3cab4ad28 100644
> --- a/drivers/media/dvb-core/dvb_demux.h
> +++ b/drivers/media/dvb-core/dvb_demux.h
> @@ -83,7 +83,7 @@ struct dvb_demux_feed {
> u8 *buffer;
> int buffer_size;
>
> - struct timespec timeout;
> + ktime_t timeout;
> struct dvb_demux_filter *filter;
>
> int ts_type;
> @@ -134,7 +134,7 @@ struct dvb_demux {
>
> uint8_t *cnt_storage; /* for TS continuity check */
>
> - struct timespec speed_last_time; /* for TS speed check */
> + ktime_t speed_last_time; /* for TS speed check */
> uint32_t speed_pkts_cnt; /* for TS speed check */
> };
>
> diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
> index b81e026edab3..df3ba15c007e 100644
> --- a/drivers/media/dvb-core/dvb_net.c
> +++ b/drivers/media/dvb-core/dvb_net.c
> @@ -998,7 +998,7 @@ static int dvb_net_feed_start(struct net_device *dev)
> netdev_dbg(dev, "start filtering\n");
> priv->secfeed->start_filtering(priv->secfeed);
> } else if (priv->feedtype == DVB_NET_FEEDTYPE_ULE) {
> - struct timespec timeout = { 0, 10000000 }; // 10 msec
> + ktime_t timeout = ns_to_ktime(10 * NSEC_PER_MSEC);
>
> /* we have payloads encapsulated in TS */
> netdev_dbg(dev, "alloc tsfeed\n");
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/7] [media] dvb: use ktime_t for internal timeout
@ 2015-09-15 17:55 ` Andreas Oberritter
0 siblings, 0 replies; 42+ messages in thread
From: Andreas Oberritter @ 2015-09-15 17:55 UTC (permalink / raw)
To: Arnd Bergmann, linux-media
Cc: y2038, linux-api, linux-samsung-soc, linux-kernel, Mauro Carvalho Chehab
Hello Arnd,
On 15.09.2015 17:49, Arnd Bergmann wrote:
> The dvb demuxer code uses a 'struct timespec' to pass a timeout
> as absolute time. This will cause problems on 32-bit architectures
> in 2038 when time_t overflows, and it is racy with a concurrent
> settimeofday() call.
>
> This patch changes the code to use ktime_get() instead, using
> the monotonic time base to avoid both the race and the overflow.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/media/dvb-core/demux.h | 2 +-
> drivers/media/dvb-core/dmxdev.c | 2 +-
> drivers/media/dvb-core/dvb_demux.c | 17 ++++++-----------
> drivers/media/dvb-core/dvb_demux.h | 4 ++--
> drivers/media/dvb-core/dvb_net.c | 2 +-
> 5 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/dvb-core/demux.h b/drivers/media/dvb-core/demux.h
> index 833191bcd810..d8e2b1213bef 100644
> --- a/drivers/media/dvb-core/demux.h
> +++ b/drivers/media/dvb-core/demux.h
> @@ -92,7 +92,7 @@ struct dmx_ts_feed {
> int type,
> enum dmx_ts_pes pes_type,
> size_t circular_buffer_size,
> - struct timespec timeout);
> + ktime_t timeout);
> int (*start_filtering) (struct dmx_ts_feed* feed);
> int (*stop_filtering) (struct dmx_ts_feed* feed);
> };
> diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
> index d0e3f9d85f34..0d20b379eeec 100644
> --- a/drivers/media/dvb-core/dmxdev.c
> +++ b/drivers/media/dvb-core/dmxdev.c
> @@ -558,7 +558,7 @@ static int dvb_dmxdev_start_feed(struct dmxdev *dmxdev,
> struct dmxdev_filter *filter,
> struct dmxdev_feed *feed)
> {
> - struct timespec timeout = { 0 };
> + ktime_t timeout = ktime_set(0, 0);
> struct dmx_pes_filter_params *para = &filter->params.pes;
> dmx_output_t otype;
> int ret;
> diff --git a/drivers/media/dvb-core/dvb_demux.c b/drivers/media/dvb-core/dvb_demux.c
> index 6c7ff0cdcd32..d83dd0eb5757 100644
> --- a/drivers/media/dvb-core/dvb_demux.c
> +++ b/drivers/media/dvb-core/dvb_demux.c
> @@ -399,28 +399,23 @@ static void dvb_dmx_swfilter_packet(struct dvb_demux *demux, const u8 *buf)
> int dvr_done = 0;
>
> if (dvb_demux_speedcheck) {
> - struct timespec cur_time, delta_time;
> + ktime_t cur_time;
> u64 speed_bytes, speed_timedelta;
>
> demux->speed_pkts_cnt++;
>
> /* show speed every SPEED_PKTS_INTERVAL packets */
> if (!(demux->speed_pkts_cnt % SPEED_PKTS_INTERVAL)) {
> - cur_time = current_kernel_time();
> + cur_time = ktime_get();
>
> - if (demux->speed_last_time.tv_sec != 0 &&
> - demux->speed_last_time.tv_nsec != 0) {
> - delta_time = timespec_sub(cur_time,
> - demux->speed_last_time);
> + if (ktime_to_ns(demux->speed_last_time) == 0) {
if ktime_to_ns does what I think it does, then you should invert the logic.
Regards,
Andreas
> speed_bytes = (u64)demux->speed_pkts_cnt
> * 188 * 8;
> /* convert to 1024 basis */
> speed_bytes = 1000 * div64_u64(speed_bytes,
> 1024);
> - speed_timedelta =
> - (u64)timespec_to_ns(&delta_time);
> - speed_timedelta = div64_u64(speed_timedelta,
> - 1000000); /* nsec -> usec */
> + speed_timedelta = ktime_ms_delta(cur_time,
> + demux->speed_last_time);
> printk(KERN_INFO "TS speed %llu Kbits/sec \n",
> div64_u64(speed_bytes,
> speed_timedelta));
> @@ -667,7 +662,7 @@ out:
>
> static int dmx_ts_feed_set(struct dmx_ts_feed *ts_feed, u16 pid, int ts_type,
> enum dmx_ts_pes pes_type,
> - size_t circular_buffer_size, struct timespec timeout)
> + size_t circular_buffer_size, ktime_t timeout)
> {
> struct dvb_demux_feed *feed = (struct dvb_demux_feed *)ts_feed;
> struct dvb_demux *demux = feed->demux;
> diff --git a/drivers/media/dvb-core/dvb_demux.h b/drivers/media/dvb-core/dvb_demux.h
> index ae7fc33c3231..5ed3cab4ad28 100644
> --- a/drivers/media/dvb-core/dvb_demux.h
> +++ b/drivers/media/dvb-core/dvb_demux.h
> @@ -83,7 +83,7 @@ struct dvb_demux_feed {
> u8 *buffer;
> int buffer_size;
>
> - struct timespec timeout;
> + ktime_t timeout;
> struct dvb_demux_filter *filter;
>
> int ts_type;
> @@ -134,7 +134,7 @@ struct dvb_demux {
>
> uint8_t *cnt_storage; /* for TS continuity check */
>
> - struct timespec speed_last_time; /* for TS speed check */
> + ktime_t speed_last_time; /* for TS speed check */
> uint32_t speed_pkts_cnt; /* for TS speed check */
> };
>
> diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
> index b81e026edab3..df3ba15c007e 100644
> --- a/drivers/media/dvb-core/dvb_net.c
> +++ b/drivers/media/dvb-core/dvb_net.c
> @@ -998,7 +998,7 @@ static int dvb_net_feed_start(struct net_device *dev)
> netdev_dbg(dev, "start filtering\n");
> priv->secfeed->start_filtering(priv->secfeed);
> } else if (priv->feedtype == DVB_NET_FEEDTYPE_ULE) {
> - struct timespec timeout = { 0, 10000000 }; // 10 msec
> + ktime_t timeout = ns_to_ktime(10 * NSEC_PER_MSEC);
>
> /* we have payloads encapsulated in TS */
> netdev_dbg(dev, "alloc tsfeed\n");
>
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 6/7] [RFC] [media]: v4l2: introduce v4l2_timeval
2015-09-15 16:27 ` Hans Verkuil
@ 2015-09-15 20:26 ` Arnd Bergmann
-1 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 20:26 UTC (permalink / raw)
To: Hans Verkuil
Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
linux-api, linux-samsung-soc
On Tuesday 15 September 2015 18:27:19 Hans Verkuil wrote:
> On 09/15/2015 05:49 PM, Arnd Bergmann wrote:
> > The v4l2 API uses a 'struct timeval' to communicate time stamps to user
> > space. This is broken on 32-bit architectures as soon as we have a C library
> > that defines time_t as 64 bit, which then changes the structure layout of
> > struct v4l2_buffer.
> >
> > Fortunately, almost all v4l2 drivers use monotonic timestamps and call
> > v4l2_get_timestamp(), which means they don't also have a y2038 problem.
> > This means we can keep using the existing binary layout of the structure
> > and do not need to worry about defining a new kernel interface for
> > userland with 64-bit time_t.
> >
> > A possible downside of this approach is that it breaks any user space
> > that tries to assign the timeval structure returned from the kernel
> > to another timeval, or to pass a pointer to it into a function that
> > expects a timeval. Those will cause a build-time warning or error
> > that can be fixed up in a backwards compatible way.
> >
> > The alternative to this patch is to leave the structure using
> > 'struct timeval', but then we have to rework the kernel to let
> > it handle both 32-bit and 64-bit time_t for 32-bit user space
> > processes.
>
> Cool. Only this morning I was thinking about what would be needed in v4l2
> to be y2038 safe, and here it is!
Nice!
fwiw, I also have a list of drivers at
https://docs.google.com/spreadsheets/d/1HCYwHXxs48TsTb6IGUduNjQnmfRvMPzCN6T_0YiQwis/edit?usp=sharing
which lists all known files that still need changing, in case you are
wondering what else needs to be done, though it currently only covers
things that nobody so far has started working on, and I have a couple
patches on my disk that need polishing (I pushed out the v4l2 portion
of that as a start)
> > @@ -839,7 +845,7 @@ struct v4l2_buffer {
> > __u32 bytesused;
> > __u32 flags;
> > __u32 field;
> > - struct timeval timestamp;
> > + struct v4l2_timeval timestamp;
> > struct v4l2_timecode timecode;
> > __u32 sequence;
> >
> >
>
> I suspect that quite a few apps use assign the timestamp to another timeval
> struct. A quick grep in v4l-utils (which we maintain) shows at least two of
> those assignments. Ditto for xawtv3.
Ok, that is very helpful information, thanks for finding that!
> So I don't think v4l2_timeval is an option as it would break userspace too badly.
Agreed, we definitely don't want to break building user space with
existing environments, i.e. 64-bit architectures, or 32-bit architectures
with 32-bit time_t.
> An alternative to supporting a 64-bit timeval for 32-bit userspace is to make a
> new y2038-aware struct and a new set of ioctls and use this opportunity to clean
> up and extend the v4l2_buffer struct.
>
> So any 32-bit app that needs to be y2038 compliant would just use the new
> struct and ioctls.
>
> But this is something to discuss among the v4l2 developers.
Ok. We generally to require as few source level changes to user space
as possible for the conversion, and we want to make sure that when
using a 32-bit libc with 64-bit time_t, we don't accidentally get
broken interfaces (i.e. we should get a compile error whenever we
can't get it right automatically).
One aspect that makes v4l2_buffer special is that the binary format
is already clean for y2038 (once patch 4/7 "exynos4-is: use monotonic
timestamps as advertized" gets merged), and we only need to worry about
what happens when user space disagrees about the size of timeval.
Let me describe the options that I can think of here:
a) Similar to my first attempt, define a new struct v4l2_timeval, but
only use it when building with a y2038-aware libc, so we don't break
existing environments:
/* some compile-time conditional that we first need to agree on with libc */
#if __BITS_PER_TIME_T > __BITS_PER_LONG
struct v4l2_timeval { long tv_sec; long tv_usec; }
#else
#define v4l2_timeval timeval
#endif
This means that any user space that currently assumes the timestamp
member to be a 'struct timeval' has to be changed to access the members
individually, or get a build error.
The __BITS_PER_TIME_T trick has to be used in a couple of other subsystems
too, as some of them have no other way to identify an interface
b) Keep the header file unchanged, but deal with both formats of v4l2_buffer
in the kernel. Fortunately, all ioctls that pass a v4l2_buffer have
properly defined command codes, and it does not get passed using a
read/write style interface. This means we move the v4l2_buffer32
handling from v4l2-compat-ioctl32.c to v4l2-ioctl.c and add an in-kernel
v4l2_buffer64 that matches the 64-bit variant of v4l2_buffer.
This way, user space can use either definition of time_t, and the
kernel will just handle them natively.
This is going to be the most common way to handle y2038 compatibility
in device drivers, and it has the additional advantage of simplifying
the compat path.
c) As you describe above, introduce a new v4l2_buffer replacement with
a different layout that does not reference timeval. For this case, I
would recommend using a single 64-bit nanosecond timestamp that can
be generated using ktime_get_ns().
However, to avoid ambiguity with the user space definition of struct
timeval, we still have to hide the existing 'struct v4l2_buffer' from
y2038-aware user space by enclosing it in '#if __BITS_PER_TIME_T >
__BITS_PER_LONG' or similar.
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 6/7] [RFC] [media]: v4l2: introduce v4l2_timeval
@ 2015-09-15 20:26 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 20:26 UTC (permalink / raw)
To: Hans Verkuil
Cc: linux-samsung-soc, Mauro Carvalho Chehab, y2038, linux-api,
linux-kernel, linux-media
On Tuesday 15 September 2015 18:27:19 Hans Verkuil wrote:
> On 09/15/2015 05:49 PM, Arnd Bergmann wrote:
> > The v4l2 API uses a 'struct timeval' to communicate time stamps to user
> > space. This is broken on 32-bit architectures as soon as we have a C library
> > that defines time_t as 64 bit, which then changes the structure layout of
> > struct v4l2_buffer.
> >
> > Fortunately, almost all v4l2 drivers use monotonic timestamps and call
> > v4l2_get_timestamp(), which means they don't also have a y2038 problem.
> > This means we can keep using the existing binary layout of the structure
> > and do not need to worry about defining a new kernel interface for
> > userland with 64-bit time_t.
> >
> > A possible downside of this approach is that it breaks any user space
> > that tries to assign the timeval structure returned from the kernel
> > to another timeval, or to pass a pointer to it into a function that
> > expects a timeval. Those will cause a build-time warning or error
> > that can be fixed up in a backwards compatible way.
> >
> > The alternative to this patch is to leave the structure using
> > 'struct timeval', but then we have to rework the kernel to let
> > it handle both 32-bit and 64-bit time_t for 32-bit user space
> > processes.
>
> Cool. Only this morning I was thinking about what would be needed in v4l2
> to be y2038 safe, and here it is!
Nice!
fwiw, I also have a list of drivers at
https://docs.google.com/spreadsheets/d/1HCYwHXxs48TsTb6IGUduNjQnmfRvMPzCN6T_0YiQwis/edit?usp=sharing
which lists all known files that still need changing, in case you are
wondering what else needs to be done, though it currently only covers
things that nobody so far has started working on, and I have a couple
patches on my disk that need polishing (I pushed out the v4l2 portion
of that as a start)
> > @@ -839,7 +845,7 @@ struct v4l2_buffer {
> > __u32 bytesused;
> > __u32 flags;
> > __u32 field;
> > - struct timeval timestamp;
> > + struct v4l2_timeval timestamp;
> > struct v4l2_timecode timecode;
> > __u32 sequence;
> >
> >
>
> I suspect that quite a few apps use assign the timestamp to another timeval
> struct. A quick grep in v4l-utils (which we maintain) shows at least two of
> those assignments. Ditto for xawtv3.
Ok, that is very helpful information, thanks for finding that!
> So I don't think v4l2_timeval is an option as it would break userspace too badly.
Agreed, we definitely don't want to break building user space with
existing environments, i.e. 64-bit architectures, or 32-bit architectures
with 32-bit time_t.
> An alternative to supporting a 64-bit timeval for 32-bit userspace is to make a
> new y2038-aware struct and a new set of ioctls and use this opportunity to clean
> up and extend the v4l2_buffer struct.
>
> So any 32-bit app that needs to be y2038 compliant would just use the new
> struct and ioctls.
>
> But this is something to discuss among the v4l2 developers.
Ok. We generally to require as few source level changes to user space
as possible for the conversion, and we want to make sure that when
using a 32-bit libc with 64-bit time_t, we don't accidentally get
broken interfaces (i.e. we should get a compile error whenever we
can't get it right automatically).
One aspect that makes v4l2_buffer special is that the binary format
is already clean for y2038 (once patch 4/7 "exynos4-is: use monotonic
timestamps as advertized" gets merged), and we only need to worry about
what happens when user space disagrees about the size of timeval.
Let me describe the options that I can think of here:
a) Similar to my first attempt, define a new struct v4l2_timeval, but
only use it when building with a y2038-aware libc, so we don't break
existing environments:
/* some compile-time conditional that we first need to agree on with libc */
#if __BITS_PER_TIME_T > __BITS_PER_LONG
struct v4l2_timeval { long tv_sec; long tv_usec; }
#else
#define v4l2_timeval timeval
#endif
This means that any user space that currently assumes the timestamp
member to be a 'struct timeval' has to be changed to access the members
individually, or get a build error.
The __BITS_PER_TIME_T trick has to be used in a couple of other subsystems
too, as some of them have no other way to identify an interface
b) Keep the header file unchanged, but deal with both formats of v4l2_buffer
in the kernel. Fortunately, all ioctls that pass a v4l2_buffer have
properly defined command codes, and it does not get passed using a
read/write style interface. This means we move the v4l2_buffer32
handling from v4l2-compat-ioctl32.c to v4l2-ioctl.c and add an in-kernel
v4l2_buffer64 that matches the 64-bit variant of v4l2_buffer.
This way, user space can use either definition of time_t, and the
kernel will just handle them natively.
This is going to be the most common way to handle y2038 compatibility
in device drivers, and it has the additional advantage of simplifying
the compat path.
c) As you describe above, introduce a new v4l2_buffer replacement with
a different layout that does not reference timeval. For this case, I
would recommend using a single 64-bit nanosecond timestamp that can
be generated using ktime_get_ns().
However, to avoid ambiguity with the user space definition of struct
timeval, we still have to hide the existing 'struct v4l2_buffer' from
y2038-aware user space by enclosing it in '#if __BITS_PER_TIME_T >
__BITS_PER_LONG' or similar.
Arnd
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Y2038] [PATCH 7/7] [RFC] [media] introduce v4l2_timespec type for timestamps
2015-09-15 16:32 ` Hans Verkuil
@ 2015-09-15 20:27 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 20:27 UTC (permalink / raw)
To: y2038
Cc: Hans Verkuil, linux-media, linux-api, linux-samsung-soc,
linux-kernel, Mauro Carvalho Chehab
On Tuesday 15 September 2015 18:32:36 Hans Verkuil wrote:
> >
> > - ktime_get_ts(×tamp);
> > + ktime_get_ts64(×tamp);
> > + vts.tv_sec = timestamp.tv_sec;
> > + vts.tv_nsec = timestamp.tv_nsec;
>
> I prefer to take this opportunity to create a v4l2_get_timespec helper
> function, just like v4l2_get_timeval.
Ok, good idea. I'll do that once we have agreed on the ABI.
> > @@ -2088,7 +2094,7 @@ struct v4l2_event {
> > } u;
> > __u32 pending;
> > __u32 sequence;
> > - struct timespec timestamp;
> > + struct v4l2_timespec timestamp;
> > __u32 id;
> > __u32 reserved[8];
> > };
> >
>
> I think I am OK with this. This timestamp is used much more rarely and I do
> not expect this ABI change to cause any problems in userspace.
I'd still wait the outcome of the v4l2_timeval discussion though. It may
be useful for consistency to pick the same approach for both structures.
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 7/7] [RFC] [media] introduce v4l2_timespec type for timestamps
@ 2015-09-15 20:27 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 20:27 UTC (permalink / raw)
To: y2038
Cc: linux-samsung-soc, Mauro Carvalho Chehab, linux-api,
linux-kernel, Hans Verkuil, linux-media
On Tuesday 15 September 2015 18:32:36 Hans Verkuil wrote:
> >
> > - ktime_get_ts(×tamp);
> > + ktime_get_ts64(×tamp);
> > + vts.tv_sec = timestamp.tv_sec;
> > + vts.tv_nsec = timestamp.tv_nsec;
>
> I prefer to take this opportunity to create a v4l2_get_timespec helper
> function, just like v4l2_get_timeval.
Ok, good idea. I'll do that once we have agreed on the ABI.
> > @@ -2088,7 +2094,7 @@ struct v4l2_event {
> > } u;
> > __u32 pending;
> > __u32 sequence;
> > - struct timespec timestamp;
> > + struct v4l2_timespec timestamp;
> > __u32 id;
> > __u32 reserved[8];
> > };
> >
>
> I think I am OK with this. This timestamp is used much more rarely and I do
> not expect this ABI change to cause any problems in userspace.
I'd still wait the outcome of the v4l2_timeval discussion though. It may
be useful for consistency to pick the same approach for both structures.
Arnd
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Y2038] [PATCH 1/7] [media] dvb: use ktime_t for internal timeout
@ 2015-09-15 20:30 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 20:30 UTC (permalink / raw)
To: y2038
Cc: Andreas Oberritter, linux-media, linux-api, linux-samsung-soc,
linux-kernel, Mauro Carvalho Chehab
On Tuesday 15 September 2015 19:55:35 Andreas Oberritter wrote:
> > /* show speed every SPEED_PKTS_INTERVAL packets */
> > if (!(demux->speed_pkts_cnt % SPEED_PKTS_INTERVAL)) {
> > - cur_time = current_kernel_time();
> > + cur_time = ktime_get();
> >
> > - if (demux->speed_last_time.tv_sec != 0 &&
> > - demux->speed_last_time.tv_nsec != 0) {
> > - delta_time = timespec_sub(cur_time,
> > - demux->speed_last_time);
> > + if (ktime_to_ns(demux->speed_last_time) == 0) {
>
> if ktime_to_ns does what I think it does, then you should invert the logic.
Thanks for taking a critical look here, you are absolutely right, and I've
now fixed it.
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Y2038] [PATCH 1/7] [media] dvb: use ktime_t for internal timeout
@ 2015-09-15 20:30 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-15 20:30 UTC (permalink / raw)
To: y2038-cunTk1MwBs8s++Sfvej+rw
Cc: Andreas Oberritter, linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mauro Carvalho Chehab
On Tuesday 15 September 2015 19:55:35 Andreas Oberritter wrote:
> > /* show speed every SPEED_PKTS_INTERVAL packets */
> > if (!(demux->speed_pkts_cnt % SPEED_PKTS_INTERVAL)) {
> > - cur_time = current_kernel_time();
> > + cur_time = ktime_get();
> >
> > - if (demux->speed_last_time.tv_sec != 0 &&
> > - demux->speed_last_time.tv_nsec != 0) {
> > - delta_time = timespec_sub(cur_time,
> > - demux->speed_last_time);
> > + if (ktime_to_ns(demux->speed_last_time) == 0) {
>
> if ktime_to_ns does what I think it does, then you should invert the logic.
Thanks for taking a critical look here, you are absolutely right, and I've
now fixed it.
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 6/7] [RFC] [media]: v4l2: introduce v4l2_timeval
2015-09-15 20:26 ` Arnd Bergmann
(?)
@ 2015-09-16 6:51 ` Hans Verkuil
2015-09-16 7:56 ` Arnd Bergmann
-1 siblings, 1 reply; 42+ messages in thread
From: Hans Verkuil @ 2015-09-16 6:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
linux-api, linux-samsung-soc
On 09/15/2015 10:26 PM, Arnd Bergmann wrote:
> On Tuesday 15 September 2015 18:27:19 Hans Verkuil wrote:
>> On 09/15/2015 05:49 PM, Arnd Bergmann wrote:
>>> The v4l2 API uses a 'struct timeval' to communicate time stamps to user
>>> space. This is broken on 32-bit architectures as soon as we have a C library
>>> that defines time_t as 64 bit, which then changes the structure layout of
>>> struct v4l2_buffer.
>>>
>>> Fortunately, almost all v4l2 drivers use monotonic timestamps and call
>>> v4l2_get_timestamp(), which means they don't also have a y2038 problem.
>>> This means we can keep using the existing binary layout of the structure
>>> and do not need to worry about defining a new kernel interface for
>>> userland with 64-bit time_t.
>>>
>>> A possible downside of this approach is that it breaks any user space
>>> that tries to assign the timeval structure returned from the kernel
>>> to another timeval, or to pass a pointer to it into a function that
>>> expects a timeval. Those will cause a build-time warning or error
>>> that can be fixed up in a backwards compatible way.
>>>
>>> The alternative to this patch is to leave the structure using
>>> 'struct timeval', but then we have to rework the kernel to let
>>> it handle both 32-bit and 64-bit time_t for 32-bit user space
>>> processes.
>>
>> Cool. Only this morning I was thinking about what would be needed in v4l2
>> to be y2038 safe, and here it is!
>
> Nice!
>
> fwiw, I also have a list of drivers at
> https://docs.google.com/spreadsheets/d/1HCYwHXxs48TsTb6IGUduNjQnmfRvMPzCN6T_0YiQwis/edit?usp=sharing
> which lists all known files that still need changing, in case you are
> wondering what else needs to be done, though it currently only covers
> things that nobody so far has started working on, and I have a couple
> patches on my disk that need polishing (I pushed out the v4l2 portion
> of that as a start)
I just *thought* about it, I never said I would do it! :-)
>
>>> @@ -839,7 +845,7 @@ struct v4l2_buffer {
>>> __u32 bytesused;
>>> __u32 flags;
>>> __u32 field;
>>> - struct timeval timestamp;
>>> + struct v4l2_timeval timestamp;
>>> struct v4l2_timecode timecode;
>>> __u32 sequence;
>>>
>>>
>>
>> I suspect that quite a few apps use assign the timestamp to another timeval
>> struct. A quick grep in v4l-utils (which we maintain) shows at least two of
>> those assignments. Ditto for xawtv3.
>
> Ok, that is very helpful information, thanks for finding that!
>
>> So I don't think v4l2_timeval is an option as it would break userspace too badly.
>
> Agreed, we definitely don't want to break building user space with
> existing environments, i.e. 64-bit architectures, or 32-bit architectures
> with 32-bit time_t.
>
>> An alternative to supporting a 64-bit timeval for 32-bit userspace is to make a
>> new y2038-aware struct and a new set of ioctls and use this opportunity to clean
>> up and extend the v4l2_buffer struct.
>>
>> So any 32-bit app that needs to be y2038 compliant would just use the new
>> struct and ioctls.
>>
>> But this is something to discuss among the v4l2 developers.
>
> Ok. We generally to require as few source level changes to user space
> as possible for the conversion, and we want to make sure that when
> using a 32-bit libc with 64-bit time_t, we don't accidentally get
> broken interfaces (i.e. we should get a compile error whenever we
> can't get it right automatically).
>
> One aspect that makes v4l2_buffer special is that the binary format
> is already clean for y2038 (once patch 4/7 "exynos4-is: use monotonic
> timestamps as advertized" gets merged), and we only need to worry about
> what happens when user space disagrees about the size of timeval.
>
> Let me describe the options that I can think of here:
>
> a) Similar to my first attempt, define a new struct v4l2_timeval, but
> only use it when building with a y2038-aware libc, so we don't break
> existing environments:
>
> /* some compile-time conditional that we first need to agree on with libc */
> #if __BITS_PER_TIME_T > __BITS_PER_LONG
> struct v4l2_timeval { long tv_sec; long tv_usec; }
> #else
> #define v4l2_timeval timeval
> #endif
>
> This means that any user space that currently assumes the timestamp
> member to be a 'struct timeval' has to be changed to access the members
> individually, or get a build error.
> The __BITS_PER_TIME_T trick has to be used in a couple of other subsystems
> too, as some of them have no other way to identify an interface
I don't like this as this means some applications will compile on 64 bit or
with a non-y2038-aware libc, but fail on a 32-bit with y2038-aware libc. This
will be confusing and it may take a long time before the application developer
discovers this.
> b) Keep the header file unchanged, but deal with both formats of v4l2_buffer
> in the kernel. Fortunately, all ioctls that pass a v4l2_buffer have
> properly defined command codes, and it does not get passed using a
> read/write style interface. This means we move the v4l2_buffer32
> handling from v4l2-compat-ioctl32.c to v4l2-ioctl.c and add an in-kernel
> v4l2_buffer64 that matches the 64-bit variant of v4l2_buffer.
> This way, user space can use either definition of time_t, and the
> kernel will just handle them natively.
> This is going to be the most common way to handle y2038 compatibility
> in device drivers, and it has the additional advantage of simplifying
> the compat path.
This would work.
> c) As you describe above, introduce a new v4l2_buffer replacement with
> a different layout that does not reference timeval. For this case, I
> would recommend using a single 64-bit nanosecond timestamp that can
> be generated using ktime_get_ns().
> However, to avoid ambiguity with the user space definition of struct
> timeval, we still have to hide the existing 'struct v4l2_buffer' from
> y2038-aware user space by enclosing it in '#if __BITS_PER_TIME_T >
> __BITS_PER_LONG' or similar.
Right, and if we do that we still have the problem I describe under a). So we
would need to implement b) regardless.
In other words, choosing c) doesn't depend on y2038 and it should be decided
on its own merits.
I've proposed this as a topic to the media workshop we'll have during the Linux
Kernel Summit.
Regards,
Hans
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 6/7] [RFC] [media]: v4l2: introduce v4l2_timeval
@ 2015-09-16 7:56 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-16 7:56 UTC (permalink / raw)
To: Hans Verkuil
Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
linux-api, linux-samsung-soc
On Wednesday 16 September 2015 08:51:14 Hans Verkuil wrote:
> > a) Similar to my first attempt, define a new struct v4l2_timeval, but
> > only use it when building with a y2038-aware libc, so we don't break
> > existing environments:
> >
> > /* some compile-time conditional that we first need to agree on with libc */
> > #if __BITS_PER_TIME_T > __BITS_PER_LONG
> > struct v4l2_timeval { long tv_sec; long tv_usec; }
> > #else
> > #define v4l2_timeval timeval
> > #endif
> >
> > This means that any user space that currently assumes the timestamp
> > member to be a 'struct timeval' has to be changed to access the members
> > individually, or get a build error.
> > The __BITS_PER_TIME_T trick has to be used in a couple of other subsystems
> > too, as some of them have no other way to identify an interface
>
> I don't like this as this means some applications will compile on 64 bit or
> with a non-y2038-aware libc, but fail on a 32-bit with y2038-aware libc. This
> will be confusing and it may take a long time before the application developer
> discovers this.
Right.
> > b) Keep the header file unchanged, but deal with both formats of v4l2_buffer
> > in the kernel. Fortunately, all ioctls that pass a v4l2_buffer have
> > properly defined command codes, and it does not get passed using a
> > read/write style interface. This means we move the v4l2_buffer32
> > handling from v4l2-compat-ioctl32.c to v4l2-ioctl.c and add an in-kernel
> > v4l2_buffer64 that matches the 64-bit variant of v4l2_buffer.
> > This way, user space can use either definition of time_t, and the
> > kernel will just handle them natively.
> > This is going to be the most common way to handle y2038 compatibility
> > in device drivers, and it has the additional advantage of simplifying
> > the compat path.
>
> This would work.
Ok. So the only downside I can think of for this is that it uses a slightly
less efficient format with additional padding in it. The kernel side will
be a little ugly as I'm trying to avoid defining a generic timeval64
structure (the generic syscalls should not need one), but I'll try to
implement it first to see how it ends up.
> > c) As you describe above, introduce a new v4l2_buffer replacement with
> > a different layout that does not reference timeval. For this case, I
> > would recommend using a single 64-bit nanosecond timestamp that can
> > be generated using ktime_get_ns().
> > However, to avoid ambiguity with the user space definition of struct
> > timeval, we still have to hide the existing 'struct v4l2_buffer' from
> > y2038-aware user space by enclosing it in '#if __BITS_PER_TIME_T >
> > __BITS_PER_LONG' or similar.
>
> Right, and if we do that we still have the problem I describe under a). So we
> would need to implement b) regardless.
>
> In other words, choosing c) doesn't depend on y2038 and it should be decided
> on its own merits.
>
> I've proposed this as a topic to the media workshop we'll have during the Linux
> Kernel Summit.
Thanks, good idea. I'll be at the kernel summit, but don't plan to attend
the media workshop otherwise. If you let me know about the schedule, I can
come to this session (or ping me on IRC or hangout when it starts).
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 6/7] [RFC] [media]: v4l2: introduce v4l2_timeval
@ 2015-09-16 7:56 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-16 7:56 UTC (permalink / raw)
To: Hans Verkuil
Cc: linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
y2038-cunTk1MwBs8s++Sfvej+rw, Mauro Carvalho Chehab,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA
On Wednesday 16 September 2015 08:51:14 Hans Verkuil wrote:
> > a) Similar to my first attempt, define a new struct v4l2_timeval, but
> > only use it when building with a y2038-aware libc, so we don't break
> > existing environments:
> >
> > /* some compile-time conditional that we first need to agree on with libc */
> > #if __BITS_PER_TIME_T > __BITS_PER_LONG
> > struct v4l2_timeval { long tv_sec; long tv_usec; }
> > #else
> > #define v4l2_timeval timeval
> > #endif
> >
> > This means that any user space that currently assumes the timestamp
> > member to be a 'struct timeval' has to be changed to access the members
> > individually, or get a build error.
> > The __BITS_PER_TIME_T trick has to be used in a couple of other subsystems
> > too, as some of them have no other way to identify an interface
>
> I don't like this as this means some applications will compile on 64 bit or
> with a non-y2038-aware libc, but fail on a 32-bit with y2038-aware libc. This
> will be confusing and it may take a long time before the application developer
> discovers this.
Right.
> > b) Keep the header file unchanged, but deal with both formats of v4l2_buffer
> > in the kernel. Fortunately, all ioctls that pass a v4l2_buffer have
> > properly defined command codes, and it does not get passed using a
> > read/write style interface. This means we move the v4l2_buffer32
> > handling from v4l2-compat-ioctl32.c to v4l2-ioctl.c and add an in-kernel
> > v4l2_buffer64 that matches the 64-bit variant of v4l2_buffer.
> > This way, user space can use either definition of time_t, and the
> > kernel will just handle them natively.
> > This is going to be the most common way to handle y2038 compatibility
> > in device drivers, and it has the additional advantage of simplifying
> > the compat path.
>
> This would work.
Ok. So the only downside I can think of for this is that it uses a slightly
less efficient format with additional padding in it. The kernel side will
be a little ugly as I'm trying to avoid defining a generic timeval64
structure (the generic syscalls should not need one), but I'll try to
implement it first to see how it ends up.
> > c) As you describe above, introduce a new v4l2_buffer replacement with
> > a different layout that does not reference timeval. For this case, I
> > would recommend using a single 64-bit nanosecond timestamp that can
> > be generated using ktime_get_ns().
> > However, to avoid ambiguity with the user space definition of struct
> > timeval, we still have to hide the existing 'struct v4l2_buffer' from
> > y2038-aware user space by enclosing it in '#if __BITS_PER_TIME_T >
> > __BITS_PER_LONG' or similar.
>
> Right, and if we do that we still have the problem I describe under a). So we
> would need to implement b) regardless.
>
> In other words, choosing c) doesn't depend on y2038 and it should be decided
> on its own merits.
>
> I've proposed this as a topic to the media workshop we'll have during the Linux
> Kernel Summit.
Thanks, good idea. I'll be at the kernel summit, but don't plan to attend
the media workshop otherwise. If you let me know about the schedule, I can
come to this session (or ping me on IRC or hangout when it starts).
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 6/7] [RFC] [media]: v4l2: introduce v4l2_timeval
2015-09-16 7:56 ` Arnd Bergmann
@ 2015-09-16 8:12 ` Hans Verkuil
-1 siblings, 0 replies; 42+ messages in thread
From: Hans Verkuil @ 2015-09-16 8:12 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
linux-api, linux-samsung-soc
On 09/16/2015 09:56 AM, Arnd Bergmann wrote:
> On Wednesday 16 September 2015 08:51:14 Hans Verkuil wrote:
>
>>> a) Similar to my first attempt, define a new struct v4l2_timeval, but
>>> only use it when building with a y2038-aware libc, so we don't break
>>> existing environments:
>>>
>>> /* some compile-time conditional that we first need to agree on with libc */
>>> #if __BITS_PER_TIME_T > __BITS_PER_LONG
>>> struct v4l2_timeval { long tv_sec; long tv_usec; }
>>> #else
>>> #define v4l2_timeval timeval
>>> #endif
>>>
>>> This means that any user space that currently assumes the timestamp
>>> member to be a 'struct timeval' has to be changed to access the members
>>> individually, or get a build error.
>>> The __BITS_PER_TIME_T trick has to be used in a couple of other subsystems
>>> too, as some of them have no other way to identify an interface
>>
>> I don't like this as this means some applications will compile on 64 bit or
>> with a non-y2038-aware libc, but fail on a 32-bit with y2038-aware libc. This
>> will be confusing and it may take a long time before the application developer
>> discovers this.
>
> Right.
>
>>> b) Keep the header file unchanged, but deal with both formats of v4l2_buffer
>>> in the kernel. Fortunately, all ioctls that pass a v4l2_buffer have
>>> properly defined command codes, and it does not get passed using a
>>> read/write style interface. This means we move the v4l2_buffer32
>>> handling from v4l2-compat-ioctl32.c to v4l2-ioctl.c and add an in-kernel
>>> v4l2_buffer64 that matches the 64-bit variant of v4l2_buffer.
>>> This way, user space can use either definition of time_t, and the
>>> kernel will just handle them natively.
>>> This is going to be the most common way to handle y2038 compatibility
>>> in device drivers, and it has the additional advantage of simplifying
>>> the compat path.
>>
>> This would work.
>
> Ok. So the only downside I can think of for this is that it uses a slightly
> less efficient format with additional padding in it. The kernel side will
> be a little ugly as I'm trying to avoid defining a generic timeval64
> structure (the generic syscalls should not need one), but I'll try to
> implement it first to see how it ends up.
>
>>> c) As you describe above, introduce a new v4l2_buffer replacement with
>>> a different layout that does not reference timeval. For this case, I
>>> would recommend using a single 64-bit nanosecond timestamp that can
>>> be generated using ktime_get_ns().
>>> However, to avoid ambiguity with the user space definition of struct
>>> timeval, we still have to hide the existing 'struct v4l2_buffer' from
>>> y2038-aware user space by enclosing it in '#if __BITS_PER_TIME_T >
>>> __BITS_PER_LONG' or similar.
>>
>> Right, and if we do that we still have the problem I describe under a). So we
>> would need to implement b) regardless.
>>
>> In other words, choosing c) doesn't depend on y2038 and it should be decided
>> on its own merits.
>>
>> I've proposed this as a topic to the media workshop we'll have during the Linux
>> Kernel Summit.
>
> Thanks, good idea. I'll be at the kernel summit, but don't plan to attend
> the media workshop otherwise. If you let me know about the schedule, I can
> come to this session (or ping me on IRC or hangout when it starts).
Are you also attending the ELCE in Dublin? We could have a quick talk there.
I think the discussion whether to switch to a new v4l2_buffer struct isn't really
dependent on anything y2038.
Regards,
Hans
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 6/7] [RFC] [media]: v4l2: introduce v4l2_timeval
@ 2015-09-16 8:12 ` Hans Verkuil
0 siblings, 0 replies; 42+ messages in thread
From: Hans Verkuil @ 2015-09-16 8:12 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-samsung-soc, Mauro Carvalho Chehab, y2038, linux-api,
linux-kernel, linux-media
On 09/16/2015 09:56 AM, Arnd Bergmann wrote:
> On Wednesday 16 September 2015 08:51:14 Hans Verkuil wrote:
>
>>> a) Similar to my first attempt, define a new struct v4l2_timeval, but
>>> only use it when building with a y2038-aware libc, so we don't break
>>> existing environments:
>>>
>>> /* some compile-time conditional that we first need to agree on with libc */
>>> #if __BITS_PER_TIME_T > __BITS_PER_LONG
>>> struct v4l2_timeval { long tv_sec; long tv_usec; }
>>> #else
>>> #define v4l2_timeval timeval
>>> #endif
>>>
>>> This means that any user space that currently assumes the timestamp
>>> member to be a 'struct timeval' has to be changed to access the members
>>> individually, or get a build error.
>>> The __BITS_PER_TIME_T trick has to be used in a couple of other subsystems
>>> too, as some of them have no other way to identify an interface
>>
>> I don't like this as this means some applications will compile on 64 bit or
>> with a non-y2038-aware libc, but fail on a 32-bit with y2038-aware libc. This
>> will be confusing and it may take a long time before the application developer
>> discovers this.
>
> Right.
>
>>> b) Keep the header file unchanged, but deal with both formats of v4l2_buffer
>>> in the kernel. Fortunately, all ioctls that pass a v4l2_buffer have
>>> properly defined command codes, and it does not get passed using a
>>> read/write style interface. This means we move the v4l2_buffer32
>>> handling from v4l2-compat-ioctl32.c to v4l2-ioctl.c and add an in-kernel
>>> v4l2_buffer64 that matches the 64-bit variant of v4l2_buffer.
>>> This way, user space can use either definition of time_t, and the
>>> kernel will just handle them natively.
>>> This is going to be the most common way to handle y2038 compatibility
>>> in device drivers, and it has the additional advantage of simplifying
>>> the compat path.
>>
>> This would work.
>
> Ok. So the only downside I can think of for this is that it uses a slightly
> less efficient format with additional padding in it. The kernel side will
> be a little ugly as I'm trying to avoid defining a generic timeval64
> structure (the generic syscalls should not need one), but I'll try to
> implement it first to see how it ends up.
>
>>> c) As you describe above, introduce a new v4l2_buffer replacement with
>>> a different layout that does not reference timeval. For this case, I
>>> would recommend using a single 64-bit nanosecond timestamp that can
>>> be generated using ktime_get_ns().
>>> However, to avoid ambiguity with the user space definition of struct
>>> timeval, we still have to hide the existing 'struct v4l2_buffer' from
>>> y2038-aware user space by enclosing it in '#if __BITS_PER_TIME_T >
>>> __BITS_PER_LONG' or similar.
>>
>> Right, and if we do that we still have the problem I describe under a). So we
>> would need to implement b) regardless.
>>
>> In other words, choosing c) doesn't depend on y2038 and it should be decided
>> on its own merits.
>>
>> I've proposed this as a topic to the media workshop we'll have during the Linux
>> Kernel Summit.
>
> Thanks, good idea. I'll be at the kernel summit, but don't plan to attend
> the media workshop otherwise. If you let me know about the schedule, I can
> come to this session (or ping me on IRC or hangout when it starts).
Are you also attending the ELCE in Dublin? We could have a quick talk there.
I think the discussion whether to switch to a new v4l2_buffer struct isn't really
dependent on anything y2038.
Regards,
Hans
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 6/7] [RFC] [media]: v4l2: introduce v4l2_timeval
@ 2015-09-16 8:40 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-16 8:40 UTC (permalink / raw)
To: Hans Verkuil
Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
linux-api, linux-samsung-soc
On Wednesday 16 September 2015 10:12:00 Hans Verkuil wrote:
>
> Are you also attending the ELCE in Dublin? We could have a quick talk there.
> I think the discussion whether to switch to a new v4l2_buffer struct isn't really
> dependent on anything y2038.
No, unfortunately I won't be there.
Concerning a v4l2_buffer, I agree we should treat that as a completely independent
topic. The problem at hand for y2038 is only about we expect to happen when someone
compiles source code that uses the existing v4l2_buffer (and v4l2_event) with a
y2038-aware libc.
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 6/7] [RFC] [media]: v4l2: introduce v4l2_timeval
@ 2015-09-16 8:40 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2015-09-16 8:40 UTC (permalink / raw)
To: Hans Verkuil
Cc: linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
y2038-cunTk1MwBs8s++Sfvej+rw, Mauro Carvalho Chehab,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA
On Wednesday 16 September 2015 10:12:00 Hans Verkuil wrote:
>
> Are you also attending the ELCE in Dublin? We could have a quick talk there.
> I think the discussion whether to switch to a new v4l2_buffer struct isn't really
> dependent on anything y2038.
No, unfortunately I won't be there.
Concerning a v4l2_buffer, I agree we should treat that as a completely independent
topic. The problem at hand for y2038 is only about we expect to happen when someone
compiles source code that uses the existing v4l2_buffer (and v4l2_event) with a
y2038-aware libc.
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/7] [media] exynos4-is: use monotonic timestamps as advertized
@ 2015-09-16 8:55 ` Sylwester Nawrocki
0 siblings, 0 replies; 42+ messages in thread
From: Sylwester Nawrocki @ 2015-09-16 8:55 UTC (permalink / raw)
To: Arnd Bergmann, linux-media
Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api, linux-samsung-soc
On 15/09/15 17:49, Arnd Bergmann wrote:
> The exynos4 fimc capture driver claims to use monotonic
> timestamps but calls ktime_get_real_ts(). This is both
> an incorrect API use, and a bad idea because of the y2038
> problem and the fact that the wall clock time is not reliable
> for timestamps across suspend or settimeofday().
>
> This changes the driver to use the normal v4l2_get_timestamp()
> function like all other drivers.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/7] [media] exynos4-is: use monotonic timestamps as advertized
@ 2015-09-16 8:55 ` Sylwester Nawrocki
0 siblings, 0 replies; 42+ messages in thread
From: Sylwester Nawrocki @ 2015-09-16 8:55 UTC (permalink / raw)
To: Arnd Bergmann, linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
y2038-cunTk1MwBs8s++Sfvej+rw, Mauro Carvalho Chehab,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA
On 15/09/15 17:49, Arnd Bergmann wrote:
> The exynos4 fimc capture driver claims to use monotonic
> timestamps but calls ktime_get_real_ts(). This is both
> an incorrect API use, and a bad idea because of the y2038
> problem and the fact that the wall clock time is not reliable
> for timestamps across suspend or settimeofday().
>
> This changes the driver to use the normal v4l2_get_timestamp()
> function like all other drivers.
>
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Acked-by: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 5/7] [media] use v4l2_get_timestamp where possible
@ 2015-09-16 8:57 ` Sylwester Nawrocki
0 siblings, 0 replies; 42+ messages in thread
From: Sylwester Nawrocki @ 2015-09-16 8:57 UTC (permalink / raw)
To: Arnd Bergmann, linux-media
Cc: linux-samsung-soc, Mauro Carvalho Chehab, y2038, linux-api, linux-kernel
On 15/09/15 17:49, Arnd Bergmann wrote:
> This is a preparation for a change to the type of v4l2 timestamps.
> v4l2_get_timestamp() is a helper function that reads the monotonic
> time and stores it into a 'struct timeval'. Multiple drivers implement
> the same thing themselves for historic reasons.
>
> Changing them all to use v4l2_get_timestamp() is more consistent
> and reduces the amount of code duplication, and most importantly
> simplifies the following changes.
>
> If desired, this patch can easily be split up into one patch per
> driver.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
For:
drivers/media/platform/exynos4-is/fimc-lite.c
drivers/media/platform/s3c-camif/camif-capture.c
Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 5/7] [media] use v4l2_get_timestamp where possible
@ 2015-09-16 8:57 ` Sylwester Nawrocki
0 siblings, 0 replies; 42+ messages in thread
From: Sylwester Nawrocki @ 2015-09-16 8:57 UTC (permalink / raw)
To: Arnd Bergmann, linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Mauro Carvalho Chehab,
y2038-cunTk1MwBs8s++Sfvej+rw, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On 15/09/15 17:49, Arnd Bergmann wrote:
> This is a preparation for a change to the type of v4l2 timestamps.
> v4l2_get_timestamp() is a helper function that reads the monotonic
> time and stores it into a 'struct timeval'. Multiple drivers implement
> the same thing themselves for historic reasons.
>
> Changing them all to use v4l2_get_timestamp() is more consistent
> and reduces the amount of code duplication, and most importantly
> simplifies the following changes.
>
> If desired, this patch can easily be split up into one patch per
> driver.
>
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> ---
For:
drivers/media/platform/exynos4-is/fimc-lite.c
drivers/media/platform/s3c-camif/camif-capture.c
Acked-by: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [3/7,media] dvb: don't use 'time_t' in event ioctl
2015-09-15 15:49 ` Arnd Bergmann
(?)
@ 2017-08-28 15:32 ` Eugene Syromiatnikov
2017-08-30 20:25 ` Arnd Bergmann
-1 siblings, 1 reply; 42+ messages in thread
From: Eugene Syromiatnikov @ 2017-08-28 15:32 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
linux-api, linux-samsung-soc, ldv, glebfm
On Tue, Sep 15, 2015 at 05:49:04PM +0200, Arnd Bergmann wrote:
> 'struct video_event' is used for the VIDEO_GET_EVENT ioctl, implemented
> by drivers/media/pci/ivtv/ivtv-ioctl.c and
> drivers/media/pci/ttpci/av7110_av.c. The structure contains a 'time_t',
> which will be redefined in the future to be 64-bit wide, causing an
> incompatible ABI change for this ioctl.
>
> As it turns out, neither of the drivers currently sets the timestamp
> field, and it is presumably useless anyway because of the limited
> resolutions (no sub-second times). This means we can simply change
> the structure definition to use a 'long' instead of 'time_t' and
> remain compatible with all existing user space binaries when time_t
> gets changed.
>
> If anybody ever starts using this field, they have to make sure not
> to use 1970 based seconds in there, as those overflow in 2038.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> include/uapi/linux/dvb/video.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/dvb/video.h b/include/uapi/linux/dvb/video.h
> index d3d14a59d2d5..6c7f9298d7c2 100644
> --- a/include/uapi/linux/dvb/video.h
> +++ b/include/uapi/linux/dvb/video.h
> @@ -135,7 +135,8 @@ struct video_event {
> #define VIDEO_EVENT_FRAME_RATE_CHANGED 2
> #define VIDEO_EVENT_DECODER_STOPPED 3
> #define VIDEO_EVENT_VSYNC 4
> - __kernel_time_t timestamp;
> + /* unused, make sure to use atomic time for y2038 if it ever gets used */
> + long timestamp;
This change breaks x32 ABI (and possibly MIPS n32 ABI), as __kernel_time_t
there is 64 bit already:
https://sourceforge.net/p/strace/mailman/message/36015326/
Note the change in structure size from 0x20 to 0x14 for VIDEO_GET_EVENT
command in linux/x32/ioctls_inc0.h.
> union {
> video_size_t size;
> unsigned int frame_rate; /* in frames per 1000sec */
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [3/7,media] dvb: don't use 'time_t' in event ioctl
2017-08-28 15:32 ` [3/7,media] " Eugene Syromiatnikov
@ 2017-08-30 20:25 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2017-08-30 20:25 UTC (permalink / raw)
To: Eugene Syromiatnikov
Cc: Linux Media Mailing List, Linux Kernel Mailing List,
y2038 Mailman List, Mauro Carvalho Chehab, Linux API,
moderated list:ARM/SAMSUNG EXYNOS ARM ARCHITECTURES,
Dmitry V. Levin, Gleb Fotengauer-Malinovskiy
>> diff --git a/include/uapi/linux/dvb/video.h b/include/uapi/linux/dvb/video.h
>> index d3d14a59d2d5..6c7f9298d7c2 100644
>> --- a/include/uapi/linux/dvb/video.h
>> +++ b/include/uapi/linux/dvb/video.h
>> @@ -135,7 +135,8 @@ struct video_event {
>> #define VIDEO_EVENT_FRAME_RATE_CHANGED 2
>> #define VIDEO_EVENT_DECODER_STOPPED 3
>> #define VIDEO_EVENT_VSYNC 4
>> - __kernel_time_t timestamp;
>> + /* unused, make sure to use atomic time for y2038 if it ever gets used */
>> + long timestamp;
>
> This change breaks x32 ABI (and possibly MIPS n32 ABI), as __kernel_time_t
> there is 64 bit already:
> https://sourceforge.net/p/strace/mailman/message/36015326/
>
> Note the change in structure size from 0x20 to 0x14 for VIDEO_GET_EVENT
> command in linux/x32/ioctls_inc0.h.
Are you sure it worked before the change? I don't see any handler in the kernel
for the x32 compat ioctl call here, only the compat_video_event handling, so
my guess is that the change unintentionally fixes x32.
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [3/7,media] dvb: don't use 'time_t' in event ioctl
@ 2017-08-30 20:25 ` Arnd Bergmann
0 siblings, 0 replies; 42+ messages in thread
From: Arnd Bergmann @ 2017-08-30 20:25 UTC (permalink / raw)
To: Eugene Syromiatnikov
Cc: Linux Media Mailing List, Linux Kernel Mailing List,
y2038 Mailman List, Mauro Carvalho Chehab, Linux API,
moderated list:ARM/SAMSUNG EXYNOS ARM ARCHITECTURES,
Dmitry V. Levin, Gleb Fotengauer-Malinovskiy
>> diff --git a/include/uapi/linux/dvb/video.h b/include/uapi/linux/dvb/video.h
>> index d3d14a59d2d5..6c7f9298d7c2 100644
>> --- a/include/uapi/linux/dvb/video.h
>> +++ b/include/uapi/linux/dvb/video.h
>> @@ -135,7 +135,8 @@ struct video_event {
>> #define VIDEO_EVENT_FRAME_RATE_CHANGED 2
>> #define VIDEO_EVENT_DECODER_STOPPED 3
>> #define VIDEO_EVENT_VSYNC 4
>> - __kernel_time_t timestamp;
>> + /* unused, make sure to use atomic time for y2038 if it ever gets used */
>> + long timestamp;
>
> This change breaks x32 ABI (and possibly MIPS n32 ABI), as __kernel_time_t
> there is 64 bit already:
> https://sourceforge.net/p/strace/mailman/message/36015326/
>
> Note the change in structure size from 0x20 to 0x14 for VIDEO_GET_EVENT
> command in linux/x32/ioctls_inc0.h.
Are you sure it worked before the change? I don't see any handler in the kernel
for the x32 compat ioctl call here, only the compat_video_event handling, so
my guess is that the change unintentionally fixes x32.
Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [3/7,media] dvb: don't use 'time_t' in event ioctl
2017-08-30 20:25 ` Arnd Bergmann
@ 2017-08-31 13:10 ` Eugene Syromiatnikov
-1 siblings, 0 replies; 42+ messages in thread
From: Eugene Syromiatnikov @ 2017-08-31 13:10 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linux Media Mailing List, Linux Kernel Mailing List,
y2038 Mailman List, Mauro Carvalho Chehab, Linux API,
moderated list:ARM/SAMSUNG EXYNOS ARM ARCHITECTURES,
Dmitry V. Levin, Gleb Fotengauer-Malinovskiy
On Wed, Aug 30, 2017 at 10:25:01PM +0200, Arnd Bergmann wrote:
> >> diff --git a/include/uapi/linux/dvb/video.h b/include/uapi/linux/dvb/video.h
> >> index d3d14a59d2d5..6c7f9298d7c2 100644
> >> --- a/include/uapi/linux/dvb/video.h
> >> +++ b/include/uapi/linux/dvb/video.h
> >> @@ -135,7 +135,8 @@ struct video_event {
> >> #define VIDEO_EVENT_FRAME_RATE_CHANGED 2
> >> #define VIDEO_EVENT_DECODER_STOPPED 3
> >> #define VIDEO_EVENT_VSYNC 4
> >> - __kernel_time_t timestamp;
> >> + /* unused, make sure to use atomic time for y2038 if it ever gets used */
> >> + long timestamp;
> >
> > This change breaks x32 ABI (and possibly MIPS n32 ABI), as __kernel_time_t
> > there is 64 bit already:
> > https://sourceforge.net/p/strace/mailman/message/36015326/
> >
> > Note the change in structure size from 0x20 to 0x14 for VIDEO_GET_EVENT
> > command in linux/x32/ioctls_inc0.h.
>
> Are you sure it worked before the change? I don't see any handler in the kernel
> for the x32 compat ioctl call here, only the compat_video_event handling, so
> my guess is that the change unintentionally fixes x32.
Yes, you're right; unfortunately, I decided to check which ioctl handler
x32 code is using only after sending the e-mail, and now in the process
of preparing some RFC patch for the ioctl commands which have discrepancies
between x32 and compat sizes.
>
> Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [3/7,media] dvb: don't use 'time_t' in event ioctl
@ 2017-08-31 13:10 ` Eugene Syromiatnikov
0 siblings, 0 replies; 42+ messages in thread
From: Eugene Syromiatnikov @ 2017-08-31 13:10 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linux Media Mailing List, Linux Kernel Mailing List,
y2038 Mailman List, Mauro Carvalho Chehab, Linux API,
moderated list:ARM/SAMSUNG EXYNOS ARM ARCHITECTURES,
Dmitry V. Levin, Gleb Fotengauer-Malinovskiy
On Wed, Aug 30, 2017 at 10:25:01PM +0200, Arnd Bergmann wrote:
> >> diff --git a/include/uapi/linux/dvb/video.h b/include/uapi/linux/dvb/video.h
> >> index d3d14a59d2d5..6c7f9298d7c2 100644
> >> --- a/include/uapi/linux/dvb/video.h
> >> +++ b/include/uapi/linux/dvb/video.h
> >> @@ -135,7 +135,8 @@ struct video_event {
> >> #define VIDEO_EVENT_FRAME_RATE_CHANGED 2
> >> #define VIDEO_EVENT_DECODER_STOPPED 3
> >> #define VIDEO_EVENT_VSYNC 4
> >> - __kernel_time_t timestamp;
> >> + /* unused, make sure to use atomic time for y2038 if it ever gets used */
> >> + long timestamp;
> >
> > This change breaks x32 ABI (and possibly MIPS n32 ABI), as __kernel_time_t
> > there is 64 bit already:
> > https://sourceforge.net/p/strace/mailman/message/36015326/
> >
> > Note the change in structure size from 0x20 to 0x14 for VIDEO_GET_EVENT
> > command in linux/x32/ioctls_inc0.h.
>
> Are you sure it worked before the change? I don't see any handler in the kernel
> for the x32 compat ioctl call here, only the compat_video_event handling, so
> my guess is that the change unintentionally fixes x32.
Yes, you're right; unfortunately, I decided to check which ioctl handler
x32 code is using only after sending the e-mail, and now in the process
of preparing some RFC patch for the ioctl commands which have discrepancies
between x32 and compat sizes.
>
> Arnd
^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2017-08-31 13:10 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-15 15:49 [PATCH 0/7] [media] y2038 conversion for subsystem Arnd Bergmann
2015-09-15 15:49 ` Arnd Bergmann
2015-09-15 15:49 ` [PATCH 1/7] [media] dvb: use ktime_t for internal timeout Arnd Bergmann
2015-09-15 15:49 ` Arnd Bergmann
2015-09-15 17:55 ` Andreas Oberritter
2015-09-15 17:55 ` Andreas Oberritter
2015-09-15 20:30 ` [Y2038] " Arnd Bergmann
2015-09-15 20:30 ` Arnd Bergmann
2015-09-15 15:49 ` [PATCH 2/7] [media] dvb: remove unused systime() function Arnd Bergmann
2015-09-15 15:49 ` Arnd Bergmann
2015-09-15 15:49 ` [PATCH 3/7] [media] dvb: don't use 'time_t' in event ioctl Arnd Bergmann
2015-09-15 15:49 ` Arnd Bergmann
2017-08-28 15:32 ` [3/7,media] " Eugene Syromiatnikov
2017-08-30 20:25 ` Arnd Bergmann
2017-08-30 20:25 ` Arnd Bergmann
2017-08-31 13:10 ` Eugene Syromiatnikov
2017-08-31 13:10 ` Eugene Syromiatnikov
2015-09-15 15:49 ` [PATCH 4/7] [media] exynos4-is: use monotonic timestamps as advertized Arnd Bergmann
2015-09-16 8:55 ` Sylwester Nawrocki
2015-09-16 8:55 ` Sylwester Nawrocki
2015-09-15 15:49 ` [PATCH 5/7] [media] use v4l2_get_timestamp where possible Arnd Bergmann
2015-09-15 15:49 ` Arnd Bergmann
2015-09-16 8:57 ` Sylwester Nawrocki
2015-09-16 8:57 ` Sylwester Nawrocki
2015-09-15 15:49 ` [PATCH 6/7] [RFC] [media]: v4l2: introduce v4l2_timeval Arnd Bergmann
2015-09-15 15:49 ` Arnd Bergmann
2015-09-15 16:27 ` Hans Verkuil
2015-09-15 16:27 ` Hans Verkuil
2015-09-15 20:26 ` Arnd Bergmann
2015-09-15 20:26 ` Arnd Bergmann
2015-09-16 6:51 ` Hans Verkuil
2015-09-16 7:56 ` Arnd Bergmann
2015-09-16 7:56 ` Arnd Bergmann
2015-09-16 8:12 ` Hans Verkuil
2015-09-16 8:12 ` Hans Verkuil
2015-09-16 8:40 ` Arnd Bergmann
2015-09-16 8:40 ` Arnd Bergmann
2015-09-15 15:49 ` [PATCH 7/7] [RFC] [media] introduce v4l2_timespec type for timestamps Arnd Bergmann
2015-09-15 15:49 ` Arnd Bergmann
2015-09-15 16:32 ` Hans Verkuil
2015-09-15 20:27 ` [Y2038] " Arnd Bergmann
2015-09-15 20:27 ` Arnd Bergmann
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.