* Re: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps
2023-06-07 21:41 [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps Jason Gerecke
@ 2023-06-08 2:51 ` Peter Hutterer
2023-06-08 13:35 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Peter Hutterer @ 2023-06-08 2:51 UTC (permalink / raw)
To: Jason Gerecke
Cc: linux-input, linux-kernel, Benjamin Tissoires, Jiri Kosina,
Ping Cheng, Aaron Armstrong Skomra, Joshua Dickens, Jason Gerecke
On Wed, Jun 07, 2023 at 02:41:02PM -0700, Jason Gerecke wrote:
> Code which interacts with timestamps needs to use the ktime_t type
> returned by functions like ktime_get. The int type does not offer
> enough space to store these values, and attempting to use it is a
> recipe for problems. In this particular case, overflows would occur
> when calculating/storing timestamps leading to incorrect values being
> reported to userspace. In some cases these bad timestamps cause input
> handling in userspace to appear hung.
>
> Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/901
> Fixes: 17d793f3ed53 ("HID: wacom: insert timestamp to packed Bluetooth (BT) events")
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Cheers,
Peter
> ---
> drivers/hid/wacom_wac.c | 4 ++--
> drivers/hid/wacom_wac.h | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 2ccf83837134..2f16e47e4b69 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1314,7 +1314,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> struct input_dev *pen_input = wacom->pen_input;
> unsigned char *data = wacom->data;
> int number_of_valid_frames = 0;
> - int time_interval = 15000000;
> + ktime_t time_interval = 15000000;
> ktime_t time_packet_received = ktime_get();
> int i;
>
> @@ -1359,7 +1359,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> bool range = frame[0] & 0x20;
> bool invert = frame[0] & 0x10;
> int frames_number_reversed = number_of_valid_frames - i - 1;
> - int event_timestamp = time_packet_received - frames_number_reversed * time_interval;
> + ktime_t event_timestamp = time_packet_received - frames_number_reversed * time_interval;
>
> if (!valid)
> continue;
> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> index 1a40bb8c5810..ee21bb260f22 100644
> --- a/drivers/hid/wacom_wac.h
> +++ b/drivers/hid/wacom_wac.h
> @@ -324,7 +324,7 @@ struct hid_data {
> int ps_connected;
> bool pad_input_event_flag;
> unsigned short sequence_number;
> - int time_delayed;
> + ktime_t time_delayed;
> };
>
> struct wacom_remote_data {
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps
2023-06-07 21:41 [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps Jason Gerecke
2023-06-08 2:51 ` Peter Hutterer
@ 2023-06-08 13:35 ` kernel test robot
2023-06-08 14:26 ` kernel test robot
2023-06-08 21:38 ` [PATCH v2] " Jason Gerecke
3 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2023-06-08 13:35 UTC (permalink / raw)
To: Jason Gerecke, linux-input, linux-kernel, Benjamin Tissoires,
Jiri Kosina
Cc: oe-kbuild-all, Ping Cheng, Aaron Armstrong Skomra, Joshua Dickens,
Jason Gerecke
Hi Jason,
kernel test robot noticed the following build errors:
[auto build test ERROR on hid/for-next]
[also build test ERROR on linus/master v6.4-rc5 next-20230608]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jason-Gerecke/HID-wacom-Use-ktime_t-rather-than-int-when-dealing-with-timestamps/20230608-054255
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20230607214102.2113-1-jason.gerecke%40wacom.com
patch subject: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps
config: arc-randconfig-c003-20230608 (https://download.01.org/0day-ci/archive/20230608/202306082127.6r7qAALi-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 12.3.0
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git remote add hid https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
git fetch hid for-next
git checkout hid/for-next
b4 shazam https://lore.kernel.org/r/20230607214102.2113-1-jason.gerecke@wacom.com
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=arc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306082127.6r7qAALi-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "__divdi3" [drivers/hid/wacom.ko] undefined!
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps
2023-06-07 21:41 [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps Jason Gerecke
2023-06-08 2:51 ` Peter Hutterer
2023-06-08 13:35 ` kernel test robot
@ 2023-06-08 14:26 ` kernel test robot
2023-06-08 21:38 ` [PATCH v2] " Jason Gerecke
3 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2023-06-08 14:26 UTC (permalink / raw)
To: Jason Gerecke, linux-input, linux-kernel, Benjamin Tissoires,
Jiri Kosina
Cc: llvm, oe-kbuild-all, Ping Cheng, Aaron Armstrong Skomra,
Joshua Dickens, Jason Gerecke
Hi Jason,
kernel test robot noticed the following build errors:
[auto build test ERROR on hid/for-next]
[also build test ERROR on linus/master v6.4-rc5 next-20230608]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jason-Gerecke/HID-wacom-Use-ktime_t-rather-than-int-when-dealing-with-timestamps/20230608-054255
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20230607214102.2113-1-jason.gerecke%40wacom.com
patch subject: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps
config: arm-randconfig-r036-20230608 (https://download.01.org/0day-ci/archive/20230608/202306082249.cMqfu24Y-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
git remote add hid https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
git fetch hid for-next
git checkout hid/for-next
b4 shazam https://lore.kernel.org/r/20230607214102.2113-1-jason.gerecke@wacom.com
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306082249.cMqfu24Y-lkp@intel.com/
All errors (new ones prefixed by >>):
>> ld.lld: error: undefined symbol: __aeabi_ldivmod
>>> referenced by wacom_wac.c
>>> drivers/hid/wacom_wac.o:(wacom_wac_irq) in archive vmlinux.a
>>> did you mean: __aeabi_idivmod
>>> defined in: arch/arm/lib/lib.a(lib1funcs.o)
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v2] HID: wacom: Use ktime_t rather than int when dealing with timestamps
2023-06-07 21:41 [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps Jason Gerecke
` (2 preceding siblings ...)
2023-06-08 14:26 ` kernel test robot
@ 2023-06-08 21:38 ` Jason Gerecke
2023-06-21 15:18 ` Jason Gerecke
2023-06-26 14:54 ` bentiss
3 siblings, 2 replies; 9+ messages in thread
From: Jason Gerecke @ 2023-06-08 21:38 UTC (permalink / raw)
To: linux-input, linux-kernel, Benjamin Tissoires, Jiri Kosina
Cc: Ping Cheng, Aaron Armstrong Skomra, Joshua Dickens,
Peter Hutterer, Jason Gerecke, stable
Code which interacts with timestamps needs to use the ktime_t type
returned by functions like ktime_get. The int type does not offer
enough space to store these values, and attempting to use it is a
recipe for problems. In this particular case, overflows would occur
when calculating/storing timestamps leading to incorrect values being
reported to userspace. In some cases these bad timestamps cause input
handling in userspace to appear hung.
Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/901
Fixes: 17d793f3ed53 ("HID: wacom: insert timestamp to packed Bluetooth (BT) events")
CC: stable@vger.kernel.org
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
v2: Use div_u64 to perform division to deal with ARC and ARM architectures
(as found by the kernel test robot)
drivers/hid/wacom_wac.c | 6 +++---
drivers/hid/wacom_wac.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 2ccf838371343..174bf03908d7c 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1314,7 +1314,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
struct input_dev *pen_input = wacom->pen_input;
unsigned char *data = wacom->data;
int number_of_valid_frames = 0;
- int time_interval = 15000000;
+ ktime_t time_interval = 15000000;
ktime_t time_packet_received = ktime_get();
int i;
@@ -1348,7 +1348,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
if (number_of_valid_frames) {
if (wacom->hid_data.time_delayed)
time_interval = ktime_get() - wacom->hid_data.time_delayed;
- time_interval /= number_of_valid_frames;
+ time_interval = div_u64(time_interval, number_of_valid_frames);
wacom->hid_data.time_delayed = time_packet_received;
}
@@ -1359,7 +1359,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
bool range = frame[0] & 0x20;
bool invert = frame[0] & 0x10;
int frames_number_reversed = number_of_valid_frames - i - 1;
- int event_timestamp = time_packet_received - frames_number_reversed * time_interval;
+ ktime_t event_timestamp = time_packet_received - frames_number_reversed * time_interval;
if (!valid)
continue;
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 1a40bb8c5810c..ee21bb260f22f 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -324,7 +324,7 @@ struct hid_data {
int ps_connected;
bool pad_input_event_flag;
unsigned short sequence_number;
- int time_delayed;
+ ktime_t time_delayed;
};
struct wacom_remote_data {
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2] HID: wacom: Use ktime_t rather than int when dealing with timestamps
2023-06-08 21:38 ` [PATCH v2] " Jason Gerecke
@ 2023-06-21 15:18 ` Jason Gerecke
2023-06-21 16:04 ` Benjamin Tissoires
2023-06-26 14:54 ` bentiss
1 sibling, 1 reply; 9+ messages in thread
From: Jason Gerecke @ 2023-06-21 15:18 UTC (permalink / raw)
To: linux-input, linux-kernel, Benjamin Tissoires, Jiri Kosina
Cc: Ping Cheng, Aaron Armstrong Skomra, Joshua Dickens,
Peter Hutterer, Jason Gerecke, stable
Following up since no action seems to have been taken on this patch yet.
On Thu, Jun 8, 2023 at 2:38 PM Jason Gerecke <killertofu@gmail.com> wrote:
>
> Code which interacts with timestamps needs to use the ktime_t type
> returned by functions like ktime_get. The int type does not offer
> enough space to store these values, and attempting to use it is a
> recipe for problems. In this particular case, overflows would occur
> when calculating/storing timestamps leading to incorrect values being
> reported to userspace. In some cases these bad timestamps cause input
> handling in userspace to appear hung.
>
> Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/901
> Fixes: 17d793f3ed53 ("HID: wacom: insert timestamp to packed Bluetooth (BT) events")
> CC: stable@vger.kernel.org
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> ---
> v2: Use div_u64 to perform division to deal with ARC and ARM architectures
> (as found by the kernel test robot)
>
> drivers/hid/wacom_wac.c | 6 +++---
> drivers/hid/wacom_wac.h | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 2ccf838371343..174bf03908d7c 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1314,7 +1314,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> struct input_dev *pen_input = wacom->pen_input;
> unsigned char *data = wacom->data;
> int number_of_valid_frames = 0;
> - int time_interval = 15000000;
> + ktime_t time_interval = 15000000;
> ktime_t time_packet_received = ktime_get();
> int i;
>
> @@ -1348,7 +1348,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> if (number_of_valid_frames) {
> if (wacom->hid_data.time_delayed)
> time_interval = ktime_get() - wacom->hid_data.time_delayed;
> - time_interval /= number_of_valid_frames;
> + time_interval = div_u64(time_interval, number_of_valid_frames);
> wacom->hid_data.time_delayed = time_packet_received;
> }
>
> @@ -1359,7 +1359,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> bool range = frame[0] & 0x20;
> bool invert = frame[0] & 0x10;
> int frames_number_reversed = number_of_valid_frames - i - 1;
> - int event_timestamp = time_packet_received - frames_number_reversed * time_interval;
> + ktime_t event_timestamp = time_packet_received - frames_number_reversed * time_interval;
>
> if (!valid)
> continue;
> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> index 1a40bb8c5810c..ee21bb260f22f 100644
> --- a/drivers/hid/wacom_wac.h
> +++ b/drivers/hid/wacom_wac.h
> @@ -324,7 +324,7 @@ struct hid_data {
> int ps_connected;
> bool pad_input_event_flag;
> unsigned short sequence_number;
> - int time_delayed;
> + ktime_t time_delayed;
> };
>
> struct wacom_remote_data {
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] HID: wacom: Use ktime_t rather than int when dealing with timestamps
2023-06-21 15:18 ` Jason Gerecke
@ 2023-06-21 16:04 ` Benjamin Tissoires
2023-06-21 20:52 ` Jason Gerecke
0 siblings, 1 reply; 9+ messages in thread
From: Benjamin Tissoires @ 2023-06-21 16:04 UTC (permalink / raw)
To: Jason Gerecke
Cc: linux-input, linux-kernel, Jiri Kosina, Ping Cheng,
Aaron Armstrong Skomra, Joshua Dickens, Peter Hutterer,
Jason Gerecke, stable
On Wed, Jun 21, 2023 at 5:18 PM Jason Gerecke <killertofu@gmail.com> wrote:
>
> Following up since no action seems to have been taken on this patch yet.
Sorry, this went through the cracks (I seem to have a lot of cracks recently...)
>
> On Thu, Jun 8, 2023 at 2:38 PM Jason Gerecke <killertofu@gmail.com> wrote:
> >
> > Code which interacts with timestamps needs to use the ktime_t type
> > returned by functions like ktime_get. The int type does not offer
> > enough space to store these values, and attempting to use it is a
> > recipe for problems. In this particular case, overflows would occur
> > when calculating/storing timestamps leading to incorrect values being
> > reported to userspace. In some cases these bad timestamps cause input
> > handling in userspace to appear hung.
I have to ask, is this something we should consider writing a test for? :)
Also, you are missing the rev-by from Peter, not sure if the tools
will pick it up automatically then.
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
> >
> > Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/901
> > Fixes: 17d793f3ed53 ("HID: wacom: insert timestamp to packed Bluetooth (BT) events")
> > CC: stable@vger.kernel.org
> > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> > ---
> > v2: Use div_u64 to perform division to deal with ARC and ARM architectures
> > (as found by the kernel test robot)
> >
> > drivers/hid/wacom_wac.c | 6 +++---
> > drivers/hid/wacom_wac.h | 2 +-
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> > index 2ccf838371343..174bf03908d7c 100644
> > --- a/drivers/hid/wacom_wac.c
> > +++ b/drivers/hid/wacom_wac.c
> > @@ -1314,7 +1314,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> > struct input_dev *pen_input = wacom->pen_input;
> > unsigned char *data = wacom->data;
> > int number_of_valid_frames = 0;
> > - int time_interval = 15000000;
> > + ktime_t time_interval = 15000000;
> > ktime_t time_packet_received = ktime_get();
> > int i;
> >
> > @@ -1348,7 +1348,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> > if (number_of_valid_frames) {
> > if (wacom->hid_data.time_delayed)
> > time_interval = ktime_get() - wacom->hid_data.time_delayed;
> > - time_interval /= number_of_valid_frames;
> > + time_interval = div_u64(time_interval, number_of_valid_frames);
> > wacom->hid_data.time_delayed = time_packet_received;
> > }
> >
> > @@ -1359,7 +1359,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> > bool range = frame[0] & 0x20;
> > bool invert = frame[0] & 0x10;
> > int frames_number_reversed = number_of_valid_frames - i - 1;
> > - int event_timestamp = time_packet_received - frames_number_reversed * time_interval;
> > + ktime_t event_timestamp = time_packet_received - frames_number_reversed * time_interval;
> >
> > if (!valid)
> > continue;
> > diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> > index 1a40bb8c5810c..ee21bb260f22f 100644
> > --- a/drivers/hid/wacom_wac.h
> > +++ b/drivers/hid/wacom_wac.h
> > @@ -324,7 +324,7 @@ struct hid_data {
> > int ps_connected;
> > bool pad_input_event_flag;
> > unsigned short sequence_number;
> > - int time_delayed;
> > + ktime_t time_delayed;
> > };
> >
> > struct wacom_remote_data {
> > --
> > 2.41.0
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] HID: wacom: Use ktime_t rather than int when dealing with timestamps
2023-06-21 16:04 ` Benjamin Tissoires
@ 2023-06-21 20:52 ` Jason Gerecke
0 siblings, 0 replies; 9+ messages in thread
From: Jason Gerecke @ 2023-06-21 20:52 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: linux-input, linux-kernel, Jiri Kosina, Ping Cheng,
Aaron Armstrong Skomra, Joshua Dickens, Peter Hutterer,
Jason Gerecke, stable
On Wed, Jun 21, 2023 at 9:04 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Wed, Jun 21, 2023 at 5:18 PM Jason Gerecke <killertofu@gmail.com> wrote:
> >
> > Following up since no action seems to have been taken on this patch yet.
>
> Sorry, this went through the cracks (I seem to have a lot of cracks recently...)
>
> >
> > On Thu, Jun 8, 2023 at 2:38 PM Jason Gerecke <killertofu@gmail.com> wrote:
> > >
> > > Code which interacts with timestamps needs to use the ktime_t type
> > > returned by functions like ktime_get. The int type does not offer
> > > enough space to store these values, and attempting to use it is a
> > > recipe for problems. In this particular case, overflows would occur
> > > when calculating/storing timestamps leading to incorrect values being
> > > reported to userspace. In some cases these bad timestamps cause input
> > > handling in userspace to appear hung.
>
> I have to ask, is this something we should consider writing a test for? :)
>
Very good point! I'm happy to work on such a test.
Are you open to merging this patch without delay while I write a
testcase? I don't like the idea of this (apparent) system freeze being
affecting users any longer than absolutely necessary.
> Also, you are missing the rev-by from Peter, not sure if the tools
> will pick it up automatically then.
>
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
Oof, good catch. Apologies to Peter :)
Jason
> Cheers,
> Benjamin
>
> > >
> > > Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/901
> > > Fixes: 17d793f3ed53 ("HID: wacom: insert timestamp to packed Bluetooth (BT) events")
> > > CC: stable@vger.kernel.org
> > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> > > ---
> > > v2: Use div_u64 to perform division to deal with ARC and ARM architectures
> > > (as found by the kernel test robot)
> > >
> > > drivers/hid/wacom_wac.c | 6 +++---
> > > drivers/hid/wacom_wac.h | 2 +-
> > > 2 files changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> > > index 2ccf838371343..174bf03908d7c 100644
> > > --- a/drivers/hid/wacom_wac.c
> > > +++ b/drivers/hid/wacom_wac.c
> > > @@ -1314,7 +1314,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> > > struct input_dev *pen_input = wacom->pen_input;
> > > unsigned char *data = wacom->data;
> > > int number_of_valid_frames = 0;
> > > - int time_interval = 15000000;
> > > + ktime_t time_interval = 15000000;
> > > ktime_t time_packet_received = ktime_get();
> > > int i;
> > >
> > > @@ -1348,7 +1348,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> > > if (number_of_valid_frames) {
> > > if (wacom->hid_data.time_delayed)
> > > time_interval = ktime_get() - wacom->hid_data.time_delayed;
> > > - time_interval /= number_of_valid_frames;
> > > + time_interval = div_u64(time_interval, number_of_valid_frames);
> > > wacom->hid_data.time_delayed = time_packet_received;
> > > }
> > >
> > > @@ -1359,7 +1359,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> > > bool range = frame[0] & 0x20;
> > > bool invert = frame[0] & 0x10;
> > > int frames_number_reversed = number_of_valid_frames - i - 1;
> > > - int event_timestamp = time_packet_received - frames_number_reversed * time_interval;
> > > + ktime_t event_timestamp = time_packet_received - frames_number_reversed * time_interval;
> > >
> > > if (!valid)
> > > continue;
> > > diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> > > index 1a40bb8c5810c..ee21bb260f22f 100644
> > > --- a/drivers/hid/wacom_wac.h
> > > +++ b/drivers/hid/wacom_wac.h
> > > @@ -324,7 +324,7 @@ struct hid_data {
> > > int ps_connected;
> > > bool pad_input_event_flag;
> > > unsigned short sequence_number;
> > > - int time_delayed;
> > > + ktime_t time_delayed;
> > > };
> > >
> > > struct wacom_remote_data {
> > > --
> > > 2.41.0
> > >
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] HID: wacom: Use ktime_t rather than int when dealing with timestamps
2023-06-08 21:38 ` [PATCH v2] " Jason Gerecke
2023-06-21 15:18 ` Jason Gerecke
@ 2023-06-26 14:54 ` bentiss
1 sibling, 0 replies; 9+ messages in thread
From: bentiss @ 2023-06-26 14:54 UTC (permalink / raw)
To: linux-input, linux-kernel, Benjamin Tissoires, Jiri Kosina,
Jason Gerecke
Cc: Benjamin Tissoires, Ping Cheng, Aaron Armstrong Skomra,
Joshua Dickens, Peter Hutterer, Jason Gerecke, stable
From: Benjamin Tissoires <bentiss@kernel.org>
On Thu, 08 Jun 2023 14:38:28 -0700, Jason Gerecke wrote:
> Code which interacts with timestamps needs to use the ktime_t type
> returned by functions like ktime_get. The int type does not offer
> enough space to store these values, and attempting to use it is a
> recipe for problems. In this particular case, overflows would occur
> when calculating/storing timestamps leading to incorrect values being
> reported to userspace. In some cases these bad timestamps cause input
> handling in userspace to appear hung.
>
> [...]
Updated the "from" email from your patch.
It would be nice if you could fix your workflow (I know you well enough
to know what is your gmail address, but not having to fix this by hand
would be preferable)
Applied to hid/hid.git (for-6.5/wacom), thanks!
[1/1] HID: wacom: Use ktime_t rather than int when dealing with timestamps
https://git.kernel.org/hid/hid/c/9a6c0e28e215
Cheers,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply [flat|nested] 9+ messages in thread