[PATCH 00/16] drivers: y2038 updates

[PATCH 00/16] drivers: y2038 updates

[Arnd Bergmann]

These are updates to devidce drivers and file systems that for some
reason or another were not included in the kernel in the previous
y2038 series.

I've gone through all users of time_t again to make sure the
kernel is in a long-term maintainable state.

Posting these as a series for better organization, but each change
here is applicable standalone.

Please merge, review, ack/nack etc as you see fit. My plan is to
include any patches that don't get a reply this time around in
a future pull request, probably for linux-5.6.

As mentioned before, the full series of 90 patches is available at
https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=y2038-endgame

    Arnd

Arnd Bergmann (16):
  staging: exfat: use prandom_u32() for i_generation
  fat: use prandom_u32() for i_generation
  net: sock: use __kernel_old_timespec instead of timespec
  dlm: use SO_SNDTIMEO_NEW instead of SO_SNDTIMEO_OLD
  xtensa: ISS: avoid struct timeval
  um: ubd: use 64-bit time_t where possible
  acct: stop using get_seconds()
  tsacct: add 64-bit btime field
  netfilter: nft_meta: use 64-bit time arithmetic
  packet: clarify timestamp overflow
  quota: avoid time_t in v1_disk_dqblk definition
  hostfs: pass 64-bit timestamps to/from user space
  hfs/hfsplus: use 64-bit inode timestamps
  drm/msm: avoid using 'timespec'
  drm/etnaviv: use ktime_t for timeouts
  firewire: ohci: stop using get_seconds() for BUS_TIME

 arch/um/drivers/cow.h                         |  2 +-
 arch/um/drivers/cow_user.c                    |  7 +++--
 arch/um/drivers/ubd_kern.c                    | 10 +++----
 arch/um/include/shared/os.h                   |  2 +-
 arch/um/os-Linux/file.c                       |  2 +-
 .../platforms/iss/include/platform/simcall.h  |  4 +--
 drivers/firewire/ohci.c                       |  2 +-
 drivers/gpu/drm/etnaviv/etnaviv_drv.c         | 19 ++++++-------
 drivers/gpu/drm/etnaviv/etnaviv_drv.h         | 21 ++++++--------
 drivers/gpu/drm/etnaviv/etnaviv_gem.c         |  5 ++--
 drivers/gpu/drm/etnaviv/etnaviv_gem.h         |  2 +-
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c         |  4 +--
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h         |  4 +--
 drivers/gpu/drm/msm/msm_drv.h                 |  3 +-
 drivers/staging/exfat/exfat_super.c           |  4 +--
 fs/dlm/lowcomms.c                             |  6 ++--
 fs/fat/inode.c                                |  3 +-
 fs/hfs/hfs_fs.h                               | 26 +++++++++++++----
 fs/hfs/inode.c                                |  4 +--
 fs/hfsplus/hfsplus_fs.h                       | 26 +++++++++++++----
 fs/hfsplus/inode.c                            | 12 ++++----
 fs/hostfs/hostfs.h                            | 22 +++++++++------
 fs/hostfs/hostfs_kern.c                       | 15 ++++++----
 fs/quota/quotaio_v1.h                         |  6 ++--
 include/linux/skbuff.h                        |  7 +++--
 include/uapi/linux/acct.h                     |  2 ++
 include/uapi/linux/taskstats.h                |  6 +++-
 kernel/acct.c                                 |  4 ++-
 kernel/tsacct.c                               |  9 ++++--
 net/compat.c                                  |  2 +-
 net/ipv4/tcp.c                                | 28 +++++++++++--------
 net/netfilter/nft_meta.c                      | 10 +++----
 net/packet/af_packet.c                        | 27 +++++++++++-------
 net/socket.c                                  |  2 +-
 34 files changed, 184 insertions(+), 124 deletions(-)

-- 
2.20.0

Cc: jdike@addtoit.com
Cc: richard@nod.at
Cc: jcmvbkbc@gmail.com
Cc: stefanr@s5r6.in-berlin.de
Cc: l.stach@pengutronix.de
Cc: linux+etnaviv@armlinux.org.uk
Cc: christian.gmeiner@gmail.com
Cc: airlied@linux.ie
Cc: daniel@ffwll.ch
Cc: robdclark@gmail.com
Cc: sean@poorly.run
Cc: valdis.kletnieks@vt.edu
Cc: gregkh@linuxfoundation.org
Cc: ccaulfie@redhat.com
Cc: teigland@redhat.com
Cc: hirofumi@mail.parknet.co.jp
Cc: jack@suse.com
Cc: davem@davemloft.net
Cc: edumazet@google.com
Cc: pablo@netfilter.org
Cc: kadlec@netfilter.org
Cc: fw@strlen.de
Cc: willemb@google.com
Cc: viro@zeniv.linux.org.uk
Cc: rfontana@redhat.com
Cc: tglx@linutronix.de
Cc: linux-um@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux1394-devel@lists.sourceforge.net
Cc: etnaviv@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org>
Cc: linux-arm-msm@vger.kernel.org>
Cc: freedreno@lists.freedesktop.org>
Cc: devel@driverdev.osuosl.org>
Cc: cluster-devel@redhat.com>
Cc: linux-fsdevel@vger.kernel.org>
Cc: netdev@vger.kernel.org>
Cc: netfilter-devel@vger.kernel.org>
Cc: coreteam@netfilter.org>

[PATCH 00/16] drivers: y2038 updates

[Arnd Bergmann]

These are updates to devidce drivers and file systems that for some
reason or another were not included in the kernel in the previous
y2038 series.

I've gone through all users of time_t again to make sure the
kernel is in a long-term maintainable state.

Posting these as a series for better organization, but each change
here is applicable standalone.

Please merge, review, ack/nack etc as you see fit. My plan is to
include any patches that don't get a reply this time around in
a future pull request, probably for linux-5.6.

As mentioned before, the full series of 90 patches is available at
https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=y2038-endgame

    Arnd

Arnd Bergmann (16):
  staging: exfat: use prandom_u32() for i_generation
  fat: use prandom_u32() for i_generation
  net: sock: use __kernel_old_timespec instead of timespec
  dlm: use SO_SNDTIMEO_NEW instead of SO_SNDTIMEO_OLD
  xtensa: ISS: avoid struct timeval
  um: ubd: use 64-bit time_t where possible
  acct: stop using get_seconds()
  tsacct: add 64-bit btime field
  netfilter: nft_meta: use 64-bit time arithmetic
  packet: clarify timestamp overflow
  quota: avoid time_t in v1_disk_dqblk definition
  hostfs: pass 64-bit timestamps to/from user space
  hfs/hfsplus: use 64-bit inode timestamps
  drm/msm: avoid using 'timespec'
  drm/etnaviv: use ktime_t for timeouts
  firewire: ohci: stop using get_seconds() for BUS_TIME

 arch/um/drivers/cow.h                         |  2 +-
 arch/um/drivers/cow_user.c                    |  7 +++--
 arch/um/drivers/ubd_kern.c                    | 10 +++----
 arch/um/include/shared/os.h                   |  2 +-
 arch/um/os-Linux/file.c                       |  2 +-
 .../platforms/iss/include/platform/simcall.h  |  4 +--
 drivers/firewire/ohci.c                       |  2 +-
 drivers/gpu/drm/etnaviv/etnaviv_drv.c         | 19 ++++++-------
 drivers/gpu/drm/etnaviv/etnaviv_drv.h         | 21 ++++++--------
 drivers/gpu/drm/etnaviv/etnaviv_gem.c         |  5 ++--
 drivers/gpu/drm/etnaviv/etnaviv_gem.h         |  2 +-
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c         |  4 +--
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h         |  4 +--
 drivers/gpu/drm/msm/msm_drv.h                 |  3 +-
 drivers/staging/exfat/exfat_super.c           |  4 +--
 fs/dlm/lowcomms.c                             |  6 ++--
 fs/fat/inode.c                                |  3 +-
 fs/hfs/hfs_fs.h                               | 26 +++++++++++++----
 fs/hfs/inode.c                                |  4 +--
 fs/hfsplus/hfsplus_fs.h                       | 26 +++++++++++++----
 fs/hfsplus/inode.c                            | 12 ++++----
 fs/hostfs/hostfs.h                            | 22 +++++++++------
 fs/hostfs/hostfs_kern.c                       | 15 ++++++----
 fs/quota/quotaio_v1.h                         |  6 ++--
 include/linux/skbuff.h                        |  7 +++--
 include/uapi/linux/acct.h                     |  2 ++
 include/uapi/linux/taskstats.h                |  6 +++-
 kernel/acct.c                                 |  4 ++-
 kernel/tsacct.c                               |  9 ++++--
 net/compat.c                                  |  2 +-
 net/ipv4/tcp.c                                | 28 +++++++++++--------
 net/netfilter/nft_meta.c                      | 10 +++----
 net/packet/af_packet.c                        | 27 +++++++++++-------
 net/socket.c                                  |  2 +-
 34 files changed, 184 insertions(+), 124 deletions(-)

-- 
2.20.0

Cc: jdike@addtoit.com
Cc: richard@nod.at
Cc: jcmvbkbc@gmail.com
Cc: stefanr@s5r6.in-berlin.de
Cc: l.stach@pengutronix.de
Cc: linux+etnaviv@armlinux.org.uk
Cc: christian.gmeiner@gmail.com
Cc: airlied@linux.ie
Cc: daniel@ffwll.ch
Cc: robdclark@gmail.com
Cc: sean@poorly.run
Cc: valdis.kletnieks@vt.edu
Cc: gregkh@linuxfoundation.org
Cc: ccaulfie@redhat.com
Cc: teigland@redhat.com
Cc: hirofumi@mail.parknet.co.jp
Cc: jack@suse.com
Cc: davem@davemloft.net
Cc: edumazet@google.com
Cc: pablo@netfilter.org
Cc: kadlec@netfilter.org
Cc: fw@strlen.de
Cc: willemb@google.com
Cc: viro@zeniv.linux.org.uk
Cc: rfontana@redhat.com
Cc: tglx@linutronix.de
Cc: linux-um@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux1394-devel@lists.sourceforge.net
Cc: etnaviv@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org>
Cc: linux-arm-msm@vger.kernel.org>
Cc: freedreno@lists.freedesktop.org>
Cc: devel@driverdev.osuosl.org>
Cc: cluster-devel@redhat.com>
Cc: linux-fsdevel@vger.kernel.org>
Cc: netdev@vger.kernel.org>
Cc: netfilter-devel@vger.kernel.org>
Cc: coreteam@netfilter.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 14/16] drm/msm: avoid using 'timespec'

[Arnd Bergmann]

The timespec structure and associated interfaces are deprecated and will
be removed in the future because of the y2038 overflow.

The use of ktime_to_timespec() in timeout_to_jiffies() does not
suffer from that overflow, but is easy to avoid by just converting
the ktime_t into jiffies directly.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpu/drm/msm/msm_drv.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 71547e756e29..740bf7c70d8f 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -454,8 +454,7 @@ static inline unsigned long timeout_to_jiffies(const ktime_t *timeout)
 		remaining_jiffies = 0;
 	} else {
 		ktime_t rem = ktime_sub(*timeout, now);
-		struct timespec ts = ktime_to_timespec(rem);
-		remaining_jiffies = timespec_to_jiffies(&ts);
+		remaining_jiffies = ktime_divns(rem, NSEC_PER_SEC / HZ);
 	}
 
 	return remaining_jiffies;
-- 
2.20.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 15/16] drm/etnaviv: use ktime_t for timeouts

[Arnd Bergmann]

struct timespec is being removed from the kernel because it often leads
to code that is not y2038-safe.

In the etnaviv driver, monotonic timestamps are used, which do not suffer
from overflow, but using ktime_t still leads to better code overall.

The conversion is straightforward for the most part, except for
etnaviv_timeout_to_jiffies(), which needs to handle arguments larger
than MAX_JIFFY_OFFSET on 32-bit architectures.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.c | 19 +++++++++----------
 drivers/gpu/drm/etnaviv/etnaviv_drv.h | 21 +++++++++------------
 drivers/gpu/drm/etnaviv/etnaviv_gem.c |  5 ++---
 drivers/gpu/drm/etnaviv/etnaviv_gem.h |  2 +-
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c |  4 ++--
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h |  4 ++--
 6 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 1f9c01be40d7..1250c5e06329 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -282,16 +282,13 @@ static int etnaviv_ioctl_gem_new(struct drm_device *dev, void *data,
 			args->flags, &args->handle);
 }
 
-#define TS(t) ((struct timespec){ \
-	.tv_sec = (t).tv_sec, \
-	.tv_nsec = (t).tv_nsec \
-})
-
 static int etnaviv_ioctl_gem_cpu_prep(struct drm_device *dev, void *data,
 		struct drm_file *file)
 {
 	struct drm_etnaviv_gem_cpu_prep *args = data;
 	struct drm_gem_object *obj;
+	ktime_t timeout = ktime_set(args->timeout.tv_sec,
+				    args->timeout.tv_nsec);
 	int ret;
 
 	if (args->op & ~(ETNA_PREP_READ | ETNA_PREP_WRITE | ETNA_PREP_NOSYNC))
@@ -301,7 +298,7 @@ static int etnaviv_ioctl_gem_cpu_prep(struct drm_device *dev, void *data,
 	if (!obj)
 		return -ENOENT;
 
-	ret = etnaviv_gem_cpu_prep(obj, args->op, &TS(args->timeout));
+	ret = etnaviv_gem_cpu_prep(obj, args->op, timeout);
 
 	drm_gem_object_put_unlocked(obj);
 
@@ -354,7 +351,8 @@ static int etnaviv_ioctl_wait_fence(struct drm_device *dev, void *data,
 {
 	struct drm_etnaviv_wait_fence *args = data;
 	struct etnaviv_drm_private *priv = dev->dev_private;
-	struct timespec *timeout = &TS(args->timeout);
+	ktime_t timeout = ktime_set(args->timeout.tv_sec,
+				    args->timeout.tv_nsec);
 	struct etnaviv_gpu *gpu;
 
 	if (args->flags & ~(ETNA_WAIT_NONBLOCK))
@@ -368,7 +366,7 @@ static int etnaviv_ioctl_wait_fence(struct drm_device *dev, void *data,
 		return -ENXIO;
 
 	if (args->flags & ETNA_WAIT_NONBLOCK)
-		timeout = NULL;
+		timeout = ktime_set(0, 0);
 
 	return etnaviv_gpu_wait_fence_interruptible(gpu, args->fence,
 						    timeout);
@@ -403,7 +401,8 @@ static int etnaviv_ioctl_gem_wait(struct drm_device *dev, void *data,
 {
 	struct etnaviv_drm_private *priv = dev->dev_private;
 	struct drm_etnaviv_gem_wait *args = data;
-	struct timespec *timeout = &TS(args->timeout);
+	ktime_t timeout = ktime_set(args->timeout.tv_sec,
+				    args->timeout.tv_nsec);
 	struct drm_gem_object *obj;
 	struct etnaviv_gpu *gpu;
 	int ret;
@@ -423,7 +422,7 @@ static int etnaviv_ioctl_gem_wait(struct drm_device *dev, void *data,
 		return -ENOENT;
 
 	if (args->flags & ETNA_WAIT_NONBLOCK)
-		timeout = NULL;
+		timeout = ktime_set(0, 0);
 
 	ret = etnaviv_gem_wait_bo(gpu, obj, timeout);
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 32cfa5a48d42..57a4e247bbcf 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -60,8 +60,7 @@ struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
 int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
 void etnaviv_gem_prime_unpin(struct drm_gem_object *obj);
 void *etnaviv_gem_vmap(struct drm_gem_object *obj);
-int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op,
-		struct timespec *timeout);
+int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op, ktime_t timeout);
 int etnaviv_gem_cpu_fini(struct drm_gem_object *obj);
 void etnaviv_gem_free_object(struct drm_gem_object *obj);
 int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file,
@@ -106,22 +105,20 @@ static inline size_t size_vstruct(size_t nelem, size_t elem_size, size_t base)
  * We need to calculate the timeout in terms of number of jiffies
  * between the specified timeout and the current CLOCK_MONOTONIC time.
  */
-static inline unsigned long etnaviv_timeout_to_jiffies(
-	const struct timespec *timeout)
+static inline unsigned long etnaviv_timeout_to_jiffies(ktime_t timeout)
 {
-	struct timespec64 ts, to;
-
-	to = timespec_to_timespec64(*timeout);
-
-	ktime_get_ts64(&ts);
+	s64 remain = ktime_to_ns(ktime_sub(timeout, ktime_get()));
 
 	/* timeouts before "now" have already expired */
-	if (timespec64_compare(&to, &ts) <= 0)
+	if (remain < 0)
 		return 0;
 
-	ts = timespec64_sub(to, ts);
+#ifndef CONFIG_64BIT
+	if (remain > ((s64)MAX_JIFFY_OFFSET * NSEC_PER_SEC / HZ))
+		return MAX_JIFFY_OFFSET;
+#endif
 
-	return timespec64_to_jiffies(&ts);
+	return nsecs_to_jiffies(remain);
 }
 
 #endif /* __ETNAVIV_DRV_H__ */
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index cb1faaac380a..febe5196788e 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -372,8 +372,7 @@ static inline enum dma_data_direction etnaviv_op_to_dma_dir(u32 op)
 		return DMA_BIDIRECTIONAL;
 }
 
-int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op,
-		struct timespec *timeout)
+int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op, ktime_t timeout)
 {
 	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
 	struct drm_device *dev = obj->dev;
@@ -431,7 +430,7 @@ int etnaviv_gem_cpu_fini(struct drm_gem_object *obj)
 }
 
 int etnaviv_gem_wait_bo(struct etnaviv_gpu *gpu, struct drm_gem_object *obj,
-	struct timespec *timeout)
+			ktime_t timeout)
 {
 	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
index d6270acce619..a3461a554a6c 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
@@ -112,7 +112,7 @@ struct etnaviv_gem_submit {
 void etnaviv_submit_put(struct etnaviv_gem_submit * submit);
 
 int etnaviv_gem_wait_bo(struct etnaviv_gpu *gpu, struct drm_gem_object *obj,
-	struct timespec *timeout);
+	ktime_t timeout);
 int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags,
 	const struct etnaviv_gem_ops *ops, struct etnaviv_gem_object **res);
 void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index d47d1a8e0219..e42b1c4d902c 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1132,7 +1132,7 @@ static void event_free(struct etnaviv_gpu *gpu, unsigned int event)
  * Cmdstream submission/retirement:
  */
 int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
-	u32 id, struct timespec *timeout)
+	u32 id, ktime_t timeout)
 {
 	struct dma_fence *fence;
 	int ret;
@@ -1179,7 +1179,7 @@ int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
  * that lock in this function while waiting.
  */
 int etnaviv_gpu_wait_obj_inactive(struct etnaviv_gpu *gpu,
-	struct etnaviv_gem_object *etnaviv_obj, struct timespec *timeout)
+	struct etnaviv_gem_object *etnaviv_obj, ktime_t timeout)
 {
 	unsigned long remaining;
 	long ret;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 8f9bd4edc96a..6d352a435427 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -169,9 +169,9 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct seq_file *m);
 void etnaviv_gpu_recover_hang(struct etnaviv_gpu *gpu);
 void etnaviv_gpu_retire(struct etnaviv_gpu *gpu);
 int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
-	u32 fence, struct timespec *timeout);
+	u32 fence, ktime_t timeout);
 int etnaviv_gpu_wait_obj_inactive(struct etnaviv_gpu *gpu,
-	struct etnaviv_gem_object *etnaviv_obj, struct timespec *timeout);
+	struct etnaviv_gem_object *etnaviv_obj, ktime_t timeout);
 struct dma_fence *etnaviv_gpu_submit(struct etnaviv_gem_submit *submit);
 int etnaviv_gpu_pm_get_sync(struct etnaviv_gpu *gpu);
 void etnaviv_gpu_pm_put(struct etnaviv_gpu *gpu);
-- 
2.20.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 15/16] drm/etnaviv: use ktime_t for timeouts

[Lucas Stach]

Am Freitag, den 08.11.2019, 22:32 +0100 schrieb Arnd Bergmann:
> struct timespec is being removed from the kernel because it often leads
> to code that is not y2038-safe.
> 
> In the etnaviv driver, monotonic timestamps are used, which do not suffer
> from overflow, but using ktime_t still leads to better code overall.
> 
> The conversion is straightforward for the most part, except for
> etnaviv_timeout_to_jiffies(), which needs to handle arguments larger
> than MAX_JIFFY_OFFSET on 32-bit architectures.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c | 19 +++++++++----------
>  drivers/gpu/drm/etnaviv/etnaviv_drv.h | 21 +++++++++------------
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c |  5 ++---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.h |  2 +-
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c |  4 ++--
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.h |  4 ++--
>  6 files changed, 25 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 1f9c01be40d7..1250c5e06329 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -282,16 +282,13 @@ static int etnaviv_ioctl_gem_new(struct drm_device *dev, void *data,
>  			args->flags, &args->handle);
>  }
>  
> -#define TS(t) ((struct timespec){ \
> -	.tv_sec = (t).tv_sec, \
> -	.tv_nsec = (t).tv_nsec \
> -})
> -
>  static int etnaviv_ioctl_gem_cpu_prep(struct drm_device *dev, void *data,
>  		struct drm_file *file)
>  {
>  	struct drm_etnaviv_gem_cpu_prep *args = data;
>  	struct drm_gem_object *obj;
> +	ktime_t timeout = ktime_set(args->timeout.tv_sec,
> +				    args->timeout.tv_nsec);
>  	int ret;
>  
>  	if (args->op & ~(ETNA_PREP_READ | ETNA_PREP_WRITE | ETNA_PREP_NOSYNC))
> @@ -301,7 +298,7 @@ static int etnaviv_ioctl_gem_cpu_prep(struct drm_device *dev, void *data,
>  	if (!obj)
>  		return -ENOENT;
>  
> -	ret = etnaviv_gem_cpu_prep(obj, args->op, &TS(args->timeout));
> +	ret = etnaviv_gem_cpu_prep(obj, args->op, timeout);
>  
>  	drm_gem_object_put_unlocked(obj);
>  
> @@ -354,7 +351,8 @@ static int etnaviv_ioctl_wait_fence(struct drm_device *dev, void *data,
>  {
>  	struct drm_etnaviv_wait_fence *args = data;
>  	struct etnaviv_drm_private *priv = dev->dev_private;
> -	struct timespec *timeout = &TS(args->timeout);
> +	ktime_t timeout = ktime_set(args->timeout.tv_sec,
> +				    args->timeout.tv_nsec);
>  	struct etnaviv_gpu *gpu;
>  
>  	if (args->flags & ~(ETNA_WAIT_NONBLOCK))
> @@ -368,7 +366,7 @@ static int etnaviv_ioctl_wait_fence(struct drm_device *dev, void *data,
>  		return -ENXIO;
>  
>  	if (args->flags & ETNA_WAIT_NONBLOCK)
> -		timeout = NULL;
> +		timeout = ktime_set(0, 0);

This is a change in behavior, as far as I can see. After this change
the called internal function is not able to differentiate between a
NONBLOCK call and a blocking call with 0 timeout. The difference being
that on a busy object the NONBLOCK call will return -EBUSY, while a
blocking call will return -ETIMEDOUT.

But then CLOCK_MONOTONIC starts at 0 and should not never wrap, right?
If that's the case then we should never encounter a genuine 0 timeout
and this change would be okay.

Regards,
Lucas
>  
>  	return etnaviv_gpu_wait_fence_interruptible(gpu, args->fence,
>  						    timeout);
> @@ -403,7 +401,8 @@ static int etnaviv_ioctl_gem_wait(struct drm_device *dev, void *data,
>  {
>  	struct etnaviv_drm_private *priv = dev->dev_private;
>  	struct drm_etnaviv_gem_wait *args = data;
> -	struct timespec *timeout = &TS(args->timeout);
> +	ktime_t timeout = ktime_set(args->timeout.tv_sec,
> +				    args->timeout.tv_nsec);
>  	struct drm_gem_object *obj;
>  	struct etnaviv_gpu *gpu;
>  	int ret;
> @@ -423,7 +422,7 @@ static int etnaviv_ioctl_gem_wait(struct drm_device *dev, void *data,
>  		return -ENOENT;
>  
>  	if (args->flags & ETNA_WAIT_NONBLOCK)
> -		timeout = NULL;
> +		timeout = ktime_set(0, 0);
>  
>  	ret = etnaviv_gem_wait_bo(gpu, obj, timeout);
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> index 32cfa5a48d42..57a4e247bbcf 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> @@ -60,8 +60,7 @@ struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
>  int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
>  void etnaviv_gem_prime_unpin(struct drm_gem_object *obj);
>  void *etnaviv_gem_vmap(struct drm_gem_object *obj);
> -int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op,
> -		struct timespec *timeout);
> +int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op, ktime_t timeout);
>  int etnaviv_gem_cpu_fini(struct drm_gem_object *obj);
>  void etnaviv_gem_free_object(struct drm_gem_object *obj);
>  int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file,
> @@ -106,22 +105,20 @@ static inline size_t size_vstruct(size_t nelem, size_t elem_size, size_t base)
>   * We need to calculate the timeout in terms of number of jiffies
>   * between the specified timeout and the current CLOCK_MONOTONIC time.
>   */
> -static inline unsigned long etnaviv_timeout_to_jiffies(
> -	const struct timespec *timeout)
> +static inline unsigned long etnaviv_timeout_to_jiffies(ktime_t timeout)
>  {
> -	struct timespec64 ts, to;
> -
> -	to = timespec_to_timespec64(*timeout);
> -
> -	ktime_get_ts64(&ts);
> +	s64 remain = ktime_to_ns(ktime_sub(timeout, ktime_get()));
>  
>  	/* timeouts before "now" have already expired */
> -	if (timespec64_compare(&to, &ts) <= 0)
> +	if (remain < 0)
>  		return 0;
>  
> -	ts = timespec64_sub(to, ts);
> +#ifndef CONFIG_64BIT
> +	if (remain > ((s64)MAX_JIFFY_OFFSET * NSEC_PER_SEC / HZ))
> +		return MAX_JIFFY_OFFSET;
> +#endif
>  
> -	return timespec64_to_jiffies(&ts);
> +	return nsecs_to_jiffies(remain);
>  }
>  
>  #endif /* __ETNAVIV_DRV_H__ */
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index cb1faaac380a..febe5196788e 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -372,8 +372,7 @@ static inline enum dma_data_direction etnaviv_op_to_dma_dir(u32 op)
>  		return DMA_BIDIRECTIONAL;
>  }
>  
> -int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op,
> -		struct timespec *timeout)
> +int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op, ktime_t timeout)
>  {
>  	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
>  	struct drm_device *dev = obj->dev;
> @@ -431,7 +430,7 @@ int etnaviv_gem_cpu_fini(struct drm_gem_object *obj)
>  }
>  
>  int etnaviv_gem_wait_bo(struct etnaviv_gpu *gpu, struct drm_gem_object *obj,
> -	struct timespec *timeout)
> +			ktime_t timeout)
>  {
>  	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> index d6270acce619..a3461a554a6c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> @@ -112,7 +112,7 @@ struct etnaviv_gem_submit {
>  void etnaviv_submit_put(struct etnaviv_gem_submit * submit);
>  
>  int etnaviv_gem_wait_bo(struct etnaviv_gpu *gpu, struct drm_gem_object *obj,
> -	struct timespec *timeout);
> +	ktime_t timeout);
>  int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags,
>  	const struct etnaviv_gem_ops *ops, struct etnaviv_gem_object **res);
>  void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index d47d1a8e0219..e42b1c4d902c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1132,7 +1132,7 @@ static void event_free(struct etnaviv_gpu *gpu, unsigned int event)
>   * Cmdstream submission/retirement:
>   */
>  int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
> -	u32 id, struct timespec *timeout)
> +	u32 id, ktime_t timeout)
>  {
>  	struct dma_fence *fence;
>  	int ret;
> @@ -1179,7 +1179,7 @@ int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
>   * that lock in this function while waiting.
>   */
>  int etnaviv_gpu_wait_obj_inactive(struct etnaviv_gpu *gpu,
> -	struct etnaviv_gem_object *etnaviv_obj, struct timespec *timeout)
> +	struct etnaviv_gem_object *etnaviv_obj, ktime_t timeout)
>  {
>  	unsigned long remaining;
>  	long ret;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 8f9bd4edc96a..6d352a435427 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -169,9 +169,9 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct seq_file *m);
>  void etnaviv_gpu_recover_hang(struct etnaviv_gpu *gpu);
>  void etnaviv_gpu_retire(struct etnaviv_gpu *gpu);
>  int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
> -	u32 fence, struct timespec *timeout);
> +	u32 fence, ktime_t timeout);
>  int etnaviv_gpu_wait_obj_inactive(struct etnaviv_gpu *gpu,
> -	struct etnaviv_gem_object *etnaviv_obj, struct timespec *timeout);
> +	struct etnaviv_gem_object *etnaviv_obj, ktime_t timeout);
>  struct dma_fence *etnaviv_gpu_submit(struct etnaviv_gem_submit *submit);
>  int etnaviv_gpu_pm_get_sync(struct etnaviv_gpu *gpu);
>  void etnaviv_gpu_pm_put(struct etnaviv_gpu *gpu);

Re: [PATCH 15/16] drm/etnaviv: use ktime_t for timeouts

[Lucas Stach]

Am Freitag, den 08.11.2019, 22:32 +0100 schrieb Arnd Bergmann:
> struct timespec is being removed from the kernel because it often leads
> to code that is not y2038-safe.
> 
> In the etnaviv driver, monotonic timestamps are used, which do not suffer
> from overflow, but using ktime_t still leads to better code overall.
> 
> The conversion is straightforward for the most part, except for
> etnaviv_timeout_to_jiffies(), which needs to handle arguments larger
> than MAX_JIFFY_OFFSET on 32-bit architectures.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c | 19 +++++++++----------
>  drivers/gpu/drm/etnaviv/etnaviv_drv.h | 21 +++++++++------------
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c |  5 ++---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.h |  2 +-
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c |  4 ++--
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.h |  4 ++--
>  6 files changed, 25 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 1f9c01be40d7..1250c5e06329 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -282,16 +282,13 @@ static int etnaviv_ioctl_gem_new(struct drm_device *dev, void *data,
>  			args->flags, &args->handle);
>  }
>  
> -#define TS(t) ((struct timespec){ \
> -	.tv_sec = (t).tv_sec, \
> -	.tv_nsec = (t).tv_nsec \
> -})
> -
>  static int etnaviv_ioctl_gem_cpu_prep(struct drm_device *dev, void *data,
>  		struct drm_file *file)
>  {
>  	struct drm_etnaviv_gem_cpu_prep *args = data;
>  	struct drm_gem_object *obj;
> +	ktime_t timeout = ktime_set(args->timeout.tv_sec,
> +				    args->timeout.tv_nsec);
>  	int ret;
>  
>  	if (args->op & ~(ETNA_PREP_READ | ETNA_PREP_WRITE | ETNA_PREP_NOSYNC))
> @@ -301,7 +298,7 @@ static int etnaviv_ioctl_gem_cpu_prep(struct drm_device *dev, void *data,
>  	if (!obj)
>  		return -ENOENT;
>  
> -	ret = etnaviv_gem_cpu_prep(obj, args->op, &TS(args->timeout));
> +	ret = etnaviv_gem_cpu_prep(obj, args->op, timeout);
>  
>  	drm_gem_object_put_unlocked(obj);
>  
> @@ -354,7 +351,8 @@ static int etnaviv_ioctl_wait_fence(struct drm_device *dev, void *data,
>  {
>  	struct drm_etnaviv_wait_fence *args = data;
>  	struct etnaviv_drm_private *priv = dev->dev_private;
> -	struct timespec *timeout = &TS(args->timeout);
> +	ktime_t timeout = ktime_set(args->timeout.tv_sec,
> +				    args->timeout.tv_nsec);
>  	struct etnaviv_gpu *gpu;
>  
>  	if (args->flags & ~(ETNA_WAIT_NONBLOCK))
> @@ -368,7 +366,7 @@ static int etnaviv_ioctl_wait_fence(struct drm_device *dev, void *data,
>  		return -ENXIO;
>  
>  	if (args->flags & ETNA_WAIT_NONBLOCK)
> -		timeout = NULL;
> +		timeout = ktime_set(0, 0);

This is a change in behavior, as far as I can see. After this change
the called internal function is not able to differentiate between a
NONBLOCK call and a blocking call with 0 timeout. The difference being
that on a busy object the NONBLOCK call will return -EBUSY, while a
blocking call will return -ETIMEDOUT.

But then CLOCK_MONOTONIC starts at 0 and should not never wrap, right?
If that's the case then we should never encounter a genuine 0 timeout
and this change would be okay.

Regards,
Lucas
>  
>  	return etnaviv_gpu_wait_fence_interruptible(gpu, args->fence,
>  						    timeout);
> @@ -403,7 +401,8 @@ static int etnaviv_ioctl_gem_wait(struct drm_device *dev, void *data,
>  {
>  	struct etnaviv_drm_private *priv = dev->dev_private;
>  	struct drm_etnaviv_gem_wait *args = data;
> -	struct timespec *timeout = &TS(args->timeout);
> +	ktime_t timeout = ktime_set(args->timeout.tv_sec,
> +				    args->timeout.tv_nsec);
>  	struct drm_gem_object *obj;
>  	struct etnaviv_gpu *gpu;
>  	int ret;
> @@ -423,7 +422,7 @@ static int etnaviv_ioctl_gem_wait(struct drm_device *dev, void *data,
>  		return -ENOENT;
>  
>  	if (args->flags & ETNA_WAIT_NONBLOCK)
> -		timeout = NULL;
> +		timeout = ktime_set(0, 0);
>  
>  	ret = etnaviv_gem_wait_bo(gpu, obj, timeout);
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> index 32cfa5a48d42..57a4e247bbcf 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> @@ -60,8 +60,7 @@ struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
>  int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
>  void etnaviv_gem_prime_unpin(struct drm_gem_object *obj);
>  void *etnaviv_gem_vmap(struct drm_gem_object *obj);
> -int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op,
> -		struct timespec *timeout);
> +int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op, ktime_t timeout);
>  int etnaviv_gem_cpu_fini(struct drm_gem_object *obj);
>  void etnaviv_gem_free_object(struct drm_gem_object *obj);
>  int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file,
> @@ -106,22 +105,20 @@ static inline size_t size_vstruct(size_t nelem, size_t elem_size, size_t base)
>   * We need to calculate the timeout in terms of number of jiffies
>   * between the specified timeout and the current CLOCK_MONOTONIC time.
>   */
> -static inline unsigned long etnaviv_timeout_to_jiffies(
> -	const struct timespec *timeout)
> +static inline unsigned long etnaviv_timeout_to_jiffies(ktime_t timeout)
>  {
> -	struct timespec64 ts, to;
> -
> -	to = timespec_to_timespec64(*timeout);
> -
> -	ktime_get_ts64(&ts);
> +	s64 remain = ktime_to_ns(ktime_sub(timeout, ktime_get()));
>  
>  	/* timeouts before "now" have already expired */
> -	if (timespec64_compare(&to, &ts) <= 0)
> +	if (remain < 0)
>  		return 0;
>  
> -	ts = timespec64_sub(to, ts);
> +#ifndef CONFIG_64BIT
> +	if (remain > ((s64)MAX_JIFFY_OFFSET * NSEC_PER_SEC / HZ))
> +		return MAX_JIFFY_OFFSET;
> +#endif
>  
> -	return timespec64_to_jiffies(&ts);
> +	return nsecs_to_jiffies(remain);
>  }
>  
>  #endif /* __ETNAVIV_DRV_H__ */
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index cb1faaac380a..febe5196788e 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -372,8 +372,7 @@ static inline enum dma_data_direction etnaviv_op_to_dma_dir(u32 op)
>  		return DMA_BIDIRECTIONAL;
>  }
>  
> -int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op,
> -		struct timespec *timeout)
> +int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op, ktime_t timeout)
>  {
>  	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
>  	struct drm_device *dev = obj->dev;
> @@ -431,7 +430,7 @@ int etnaviv_gem_cpu_fini(struct drm_gem_object *obj)
>  }
>  
>  int etnaviv_gem_wait_bo(struct etnaviv_gpu *gpu, struct drm_gem_object *obj,
> -	struct timespec *timeout)
> +			ktime_t timeout)
>  {
>  	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> index d6270acce619..a3461a554a6c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> @@ -112,7 +112,7 @@ struct etnaviv_gem_submit {
>  void etnaviv_submit_put(struct etnaviv_gem_submit * submit);
>  
>  int etnaviv_gem_wait_bo(struct etnaviv_gpu *gpu, struct drm_gem_object *obj,
> -	struct timespec *timeout);
> +	ktime_t timeout);
>  int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags,
>  	const struct etnaviv_gem_ops *ops, struct etnaviv_gem_object **res);
>  void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index d47d1a8e0219..e42b1c4d902c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1132,7 +1132,7 @@ static void event_free(struct etnaviv_gpu *gpu, unsigned int event)
>   * Cmdstream submission/retirement:
>   */
>  int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
> -	u32 id, struct timespec *timeout)
> +	u32 id, ktime_t timeout)
>  {
>  	struct dma_fence *fence;
>  	int ret;
> @@ -1179,7 +1179,7 @@ int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
>   * that lock in this function while waiting.
>   */
>  int etnaviv_gpu_wait_obj_inactive(struct etnaviv_gpu *gpu,
> -	struct etnaviv_gem_object *etnaviv_obj, struct timespec *timeout)
> +	struct etnaviv_gem_object *etnaviv_obj, ktime_t timeout)
>  {
>  	unsigned long remaining;
>  	long ret;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 8f9bd4edc96a..6d352a435427 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -169,9 +169,9 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct seq_file *m);
>  void etnaviv_gpu_recover_hang(struct etnaviv_gpu *gpu);
>  void etnaviv_gpu_retire(struct etnaviv_gpu *gpu);
>  int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
> -	u32 fence, struct timespec *timeout);
> +	u32 fence, ktime_t timeout);
>  int etnaviv_gpu_wait_obj_inactive(struct etnaviv_gpu *gpu,
> -	struct etnaviv_gem_object *etnaviv_obj, struct timespec *timeout);
> +	struct etnaviv_gem_object *etnaviv_obj, ktime_t timeout);
>  struct dma_fence *etnaviv_gpu_submit(struct etnaviv_gem_submit *submit);
>  int etnaviv_gpu_pm_get_sync(struct etnaviv_gpu *gpu);
>  void etnaviv_gpu_pm_put(struct etnaviv_gpu *gpu);

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 15/16] drm/etnaviv: use ktime_t for timeouts

[Arnd Bergmann]

On Sat, Nov 9, 2019 at 12:03 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Am Freitag, den 08.11.2019, 22:32 +0100 schrieb Arnd Bergmann:
> > struct timespec is being removed from the kernel because it often leads
> > to code that is not y2038-safe.
> >
> > In the etnaviv driver, monotonic timestamps are used, which do not suffer
> > from overflow, but using ktime_t still leads to better code overall.
> >
> > The conversion is straightforward for the most part, except for
> > etnaviv_timeout_to_jiffies(), which needs to handle arguments larger
> > than MAX_JIFFY_OFFSET on 32-bit architectures.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> > @@ -368,7 +366,7 @@ static int etnaviv_ioctl_wait_fence(struct drm_device *dev, void *data,
> >               return -ENXIO;
> >
> >       if (args->flags & ETNA_WAIT_NONBLOCK)
> > -             timeout = NULL;
> > +             timeout = ktime_set(0, 0);
>
> This is a change in behavior, as far as I can see. After this change
> the called internal function is not able to differentiate between a
> NONBLOCK call and a blocking call with 0 timeout. The difference being
> that on a busy object the NONBLOCK call will return -EBUSY, while a
> blocking call will return -ETIMEDOUT.

Ah, good point. I created this patch a long time ago (cherry-picked it out
of an older branch I had done), so I don't remember how I concluded this
was a safe conversion, of if I missed that difference originally.

> But then CLOCK_MONOTONIC starts at 0 and should not never wrap, right?

Yes, that is correct.

> If that's the case then we should never encounter a genuine 0 timeout
> and this change would be okay.

That's quite likely, I'd say any program passing {0,0} as a timeout without
ETNA_WAIT_NONBLOCK is already broken, but if we leave it like that,
it would be best to describe the reasoning in the changelog.

Should I change the changelog, or change the patch to restore the
current behavior instead?

I guess I could fold the change below into my patch to make it transparent
to the application again.

      Arnd

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 1250c5e06329..162cedfb7f72 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -354,6 +354,7 @@ static int etnaviv_ioctl_wait_fence(struct
drm_device *dev, void *data,
        ktime_t timeout = ktime_set(args->timeout.tv_sec,
                                    args->timeout.tv_nsec);
        struct etnaviv_gpu *gpu;
+       int ret;

        if (args->flags & ~(ETNA_WAIT_NONBLOCK))
                return -EINVAL;
@@ -365,8 +366,12 @@ static int etnaviv_ioctl_wait_fence(struct
drm_device *dev, void *data,
        if (!gpu)
                return -ENXIO;

-       if (args->flags & ETNA_WAIT_NONBLOCK)
-               timeout = ktime_set(0, 0);
+       if (args->flags & ETNA_WAIT_NONBLOCK) {
+               ret = etnaviv_gpu_wait_fence_interruptible(gpu, args->fence,
+                                                          ktime_set(0, 0));
+
+               return (ret == -ETIMEDOUT) ? -EBUSY : ret;
+       }

        return etnaviv_gpu_wait_fence_interruptible(gpu, args->fence,
                                                    timeout);
@@ -421,10 +426,13 @@ static int etnaviv_ioctl_gem_wait(struct
drm_device *dev, void *data,
        if (!obj)
                return -ENOENT;

-       if (args->flags & ETNA_WAIT_NONBLOCK)
-               timeout = ktime_set(0, 0);
-
-       ret = etnaviv_gem_wait_bo(gpu, obj, timeout);
+       if (args->flags & ETNA_WAIT_NONBLOCK) {
+               ret = etnaviv_gem_wait_bo(gpu, obj, ktime_set(0, 0));
+               if (ret == -ETIMEDOUT)
+                       ret = -EBUSY;
+       } else {
+               ret = etnaviv_gem_wait_bo(gpu, obj, timeout);
+       }

        drm_gem_object_put_unlocked(obj);

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index e42b1c4d902c..fa6986c5a5fe 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1135,6 +1135,7 @@ int etnaviv_gpu_wait_fence_interruptible(struct
etnaviv_gpu *gpu,
        u32 id, ktime_t timeout)
 {
        struct dma_fence *fence;
+       unsigned long remaining;
        int ret;

        /*
@@ -1151,12 +1152,12 @@ int
etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
        if (!fence)
                return 0;

-       if (!timeout) {
-               /* No timeout was requested: just test for completion */
-               ret = dma_fence_is_signaled(fence) ? 0 : -EBUSY;
+       if (!timeout ||
+           (remaining = etnaviv_timeout_to_jiffies(timeout)) == 0) {
+               /* No timeout was requested, or timeout is already expired,
+                * just test for completion */
+               ret = dma_fence_is_signaled(fence) ? 0 : -ETIMEDOUT;
        } else {
-               unsigned long remaining = etnaviv_timeout_to_jiffies(timeout);
-
                ret = dma_fence_wait_timeout(fence, true, remaining);
                if (ret == 0)
                        ret = -ETIMEDOUT;
@@ -1185,7 +1186,7 @@ int etnaviv_gpu_wait_obj_inactive(struct etnaviv_gpu *gpu,
        long ret;

        if (!timeout)
-               return !is_active(etnaviv_obj) ? 0 : -EBUSY;
+               return !is_active(etnaviv_obj) ? 0 : -ETIMEDOUT;

        remaining = etnaviv_timeout_to_jiffies(timeout);
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 15/16] drm/etnaviv: use ktime_t for timeouts

[Lucas Stach]

On Sa, 2019-11-09 at 13:12 +0100, Arnd Bergmann wrote:
> On Sat, Nov 9, 2019 at 12:03 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> > Am Freitag, den 08.11.2019, 22:32 +0100 schrieb Arnd Bergmann:
> > > struct timespec is being removed from the kernel because it often leads
> > > to code that is not y2038-safe.
> > > 
> > > In the etnaviv driver, monotonic timestamps are used, which do not suffer
> > > from overflow, but using ktime_t still leads to better code overall.
> > > 
> > > The conversion is straightforward for the most part, except for
> > > etnaviv_timeout_to_jiffies(), which needs to handle arguments larger
> > > than MAX_JIFFY_OFFSET on 32-bit architectures.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > @@ -368,7 +366,7 @@ static int etnaviv_ioctl_wait_fence(struct drm_device *dev, void *data,
> > >               return -ENXIO;
> > > 
> > >       if (args->flags & ETNA_WAIT_NONBLOCK)
> > > -             timeout = NULL;
> > > +             timeout = ktime_set(0, 0);
> > 
> > This is a change in behavior, as far as I can see. After this change
> > the called internal function is not able to differentiate between a
> > NONBLOCK call and a blocking call with 0 timeout. The difference being
> > that on a busy object the NONBLOCK call will return -EBUSY, while a
> > blocking call will return -ETIMEDOUT.
> 
> Ah, good point. I created this patch a long time ago (cherry-picked it out
> of an older branch I had done), so I don't remember how I concluded this
> was a safe conversion, of if I missed that difference originally.
> 
> > But then CLOCK_MONOTONIC starts at 0 and should not never wrap, right?
> 
> Yes, that is correct.
> 
> > If that's the case then we should never encounter a genuine 0 timeout
> > and this change would be okay.
> 
> That's quite likely, I'd say any program passing {0,0} as a timeout without
> ETNA_WAIT_NONBLOCK is already broken, but if we leave it like that,
> it would be best to describe the reasoning in the changelog.
> 
> Should I change the changelog, or change the patch to restore the
> current behavior instead?
> 
> I guess I could fold the change below into my patch to make it transparent
> to the application again.

If we assume 0 to never be a valid timeout, due to monotonic clock
starting at 0 and never wrapping then I think we shouldn't introduce
any additional code complexity to fix up the return value for this
specific case. I'm not aware of any etnaviv userspace being broken in
this way to rely on the return value for an invalid timeout input.

Please just amend the commit message to mention the change in behavior
and why we think it is safe to do.

Regards,
Lucas

>       Arnd
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 1250c5e06329..162cedfb7f72 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -354,6 +354,7 @@ static int etnaviv_ioctl_wait_fence(struct
> drm_device *dev, void *data,
>         ktime_t timeout = ktime_set(args->timeout.tv_sec,
>                                     args->timeout.tv_nsec);
>         struct etnaviv_gpu *gpu;
> +       int ret;
> 
>         if (args->flags & ~(ETNA_WAIT_NONBLOCK))
>                 return -EINVAL;
> @@ -365,8 +366,12 @@ static int etnaviv_ioctl_wait_fence(struct
> drm_device *dev, void *data,
>         if (!gpu)
>                 return -ENXIO;
> 
> -       if (args->flags & ETNA_WAIT_NONBLOCK)
> -               timeout = ktime_set(0, 0);
> +       if (args->flags & ETNA_WAIT_NONBLOCK) {
> +               ret = etnaviv_gpu_wait_fence_interruptible(gpu, args->fence,
> +                                                          ktime_set(0, 0));
> +
> +               return (ret == -ETIMEDOUT) ? -EBUSY : ret;
> +       }
> 
>         return etnaviv_gpu_wait_fence_interruptible(gpu, args->fence,
>                                                     timeout);
> @@ -421,10 +426,13 @@ static int etnaviv_ioctl_gem_wait(struct
> drm_device *dev, void *data,
>         if (!obj)
>                 return -ENOENT;
> 
> -       if (args->flags & ETNA_WAIT_NONBLOCK)
> -               timeout = ktime_set(0, 0);
> -
> -       ret = etnaviv_gem_wait_bo(gpu, obj, timeout);
> +       if (args->flags & ETNA_WAIT_NONBLOCK) {
> +               ret = etnaviv_gem_wait_bo(gpu, obj, ktime_set(0, 0));
> +               if (ret == -ETIMEDOUT)
> +                       ret = -EBUSY;
> +       } else {
> +               ret = etnaviv_gem_wait_bo(gpu, obj, timeout);
> +       }
> 
>         drm_gem_object_put_unlocked(obj);
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index e42b1c4d902c..fa6986c5a5fe 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1135,6 +1135,7 @@ int etnaviv_gpu_wait_fence_interruptible(struct
> etnaviv_gpu *gpu,
>         u32 id, ktime_t timeout)
>  {
>         struct dma_fence *fence;
> +       unsigned long remaining;
>         int ret;
> 
>         /*
> @@ -1151,12 +1152,12 @@ int
> etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
>         if (!fence)
>                 return 0;
> 
> -       if (!timeout) {
> -               /* No timeout was requested: just test for completion */
> -               ret = dma_fence_is_signaled(fence) ? 0 : -EBUSY;
> +       if (!timeout ||
> +           (remaining = etnaviv_timeout_to_jiffies(timeout)) == 0) {
> +               /* No timeout was requested, or timeout is already expired,
> +                * just test for completion */
> +               ret = dma_fence_is_signaled(fence) ? 0 : -ETIMEDOUT;
>         } else {
> -               unsigned long remaining = etnaviv_timeout_to_jiffies(timeout);
> -
>                 ret = dma_fence_wait_timeout(fence, true, remaining);
>                 if (ret == 0)
>                         ret = -ETIMEDOUT;
> @@ -1185,7 +1186,7 @@ int etnaviv_gpu_wait_obj_inactive(struct etnaviv_gpu *gpu,
>         long ret;
> 
>         if (!timeout)
> -               return !is_active(etnaviv_obj) ? 0 : -EBUSY;
> +               return !is_active(etnaviv_obj) ? 0 : -ETIMEDOUT;
> 
>         remaining = etnaviv_timeout_to_jiffies(timeout);

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 15/16] drm/etnaviv: use ktime_t for timeouts

[Arnd Bergmann]

On Mon, Nov 11, 2019 at 10:55 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> >
> > > If that's the case then we should never encounter a genuine 0 timeout
> > > and this change would be okay.
> >
> > That's quite likely, I'd say any program passing {0,0} as a timeout without
> > ETNA_WAIT_NONBLOCK is already broken, but if we leave it like that,
> > it would be best to describe the reasoning in the changelog.
> >
> > Should I change the changelog, or change the patch to restore the
> > current behavior instead?
> >
> > I guess I could fold the change below into my patch to make it transparent
> > to the application again.
>
> If we assume 0 to never be a valid timeout, due to monotonic clock
> starting at 0 and never wrapping then I think we shouldn't introduce
> any additional code complexity to fix up the return value for this
> specific case. I'm not aware of any etnaviv userspace being broken in
> this way to rely on the return value for an invalid timeout input.
>
> Please just amend the commit message to mention the change in behavior
> and why we think it is safe to do.

Russell had some additional concerns that he raised on IRC,
and I did a new simpler implementation of the patch, plus a related
bugfix.

Please have a look at those.

       Arnd

Re: [PATCH 15/16] drm/etnaviv: use ktime_t for timeouts

[Arnd Bergmann]

On Mon, Nov 11, 2019 at 10:55 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> >
> > > If that's the case then we should never encounter a genuine 0 timeout
> > > and this change would be okay.
> >
> > That's quite likely, I'd say any program passing {0,0} as a timeout without
> > ETNA_WAIT_NONBLOCK is already broken, but if we leave it like that,
> > it would be best to describe the reasoning in the changelog.
> >
> > Should I change the changelog, or change the patch to restore the
> > current behavior instead?
> >
> > I guess I could fold the change below into my patch to make it transparent
> > to the application again.
>
> If we assume 0 to never be a valid timeout, due to monotonic clock
> starting at 0 and never wrapping then I think we shouldn't introduce
> any additional code complexity to fix up the return value for this
> specific case. I'm not aware of any etnaviv userspace being broken in
> this way to rely on the return value for an invalid timeout input.
>
> Please just amend the commit message to mention the change in behavior
> and why we think it is safe to do.

Russell had some additional concerns that he raised on IRC,
and I did a new simpler implementation of the patch, plus a related
bugfix.

Please have a look at those.

       Arnd
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 14/16] drm/msm: avoid using 'timespec'

[Jordan Crouse]

On Fri, Nov 08, 2019 at 10:32:52PM +0100, Arnd Bergmann wrote:
> The timespec structure and associated interfaces are deprecated and will
> be removed in the future because of the y2038 overflow.
> 
> The use of ktime_to_timespec() in timeout_to_jiffies() does not
> suffer from that overflow, but is easy to avoid by just converting
> the ktime_t into jiffies directly.

This seems good to me. y2038 changes are the best changes.

Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org>

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/gpu/drm/msm/msm_drv.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 71547e756e29..740bf7c70d8f 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -454,8 +454,7 @@ static inline unsigned long timeout_to_jiffies(const ktime_t *timeout)
>  		remaining_jiffies = 0;
>  	} else {
>  		ktime_t rem = ktime_sub(*timeout, now);
> -		struct timespec ts = ktime_to_timespec(rem);
> -		remaining_jiffies = timespec_to_jiffies(&ts);
> +		remaining_jiffies = ktime_divns(rem, NSEC_PER_SEC / HZ);
>  	}
>  
>  	return remaining_jiffies;
> -- 
> 2.20.0
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 14/16] drm/msm: avoid using 'timespec'

[Jordan Crouse]

On Fri, Nov 08, 2019 at 10:32:52PM +0100, Arnd Bergmann wrote:
> The timespec structure and associated interfaces are deprecated and will
> be removed in the future because of the y2038 overflow.
> 
> The use of ktime_to_timespec() in timeout_to_jiffies() does not
> suffer from that overflow, but is easy to avoid by just converting
> the ktime_t into jiffies directly.

This seems good to me. y2038 changes are the best changes.

Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org>

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/gpu/drm/msm/msm_drv.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 71547e756e29..740bf7c70d8f 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -454,8 +454,7 @@ static inline unsigned long timeout_to_jiffies(const ktime_t *timeout)
>  		remaining_jiffies = 0;
>  	} else {
>  		ktime_t rem = ktime_sub(*timeout, now);
> -		struct timespec ts = ktime_to_timespec(rem);
> -		remaining_jiffies = timespec_to_jiffies(&ts);
> +		remaining_jiffies = ktime_divns(rem, NSEC_PER_SEC / HZ);
>  	}
>  
>  	return remaining_jiffies;
> -- 
> 2.20.0
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel