* [PATCH v3 1/3] HID: core: reformat and reduce hid_printk macros [not found] <20190812152022.27963-1-stillcompiling@gmail.com> @ 2019-08-12 15:20 ` stillcompiling 2019-08-12 15:20 ` [PATCH v3 2/3] HID: core: Add printk_once variants to hid_warn() etc stillcompiling 2019-08-12 15:20 ` [PATCH v3 3/3] HID: core: fix dmesg flooding if report field larger than 32bit stillcompiling 2 siblings, 0 replies; 6+ messages in thread From: stillcompiling @ 2019-08-12 15:20 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires, open list:HID CORE LAYER, open list Cc: Joe Perches, Joshua Clayton From: Joshua Clayton <stillcompiling@gmail.com> Reformat hid_printk macros to use standard __VA_ARGS__ syntax. Per Joe Perches hid_printk(), hid_emerg(), hid_crit(), and hid_alert() are unlikely ever to be used. Remove them. Signed-off-by: Joshua Clayton <stillcompiling@gmail.com> diff --git a/include/linux/hid.h b/include/linux/hid.h index d770ab1a0479..e6c7efdb0458 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -1154,29 +1154,21 @@ int hid_pidff_init(struct hid_device *hid); #define hid_pidff_init NULL #endif -#define dbg_hid(format, arg...) \ +#define dbg_hid(fmt, ...) \ do { \ if (hid_debug) \ - printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \ + printk(KERN_DEBUG "%s: " fmt, __FILE__, ##__VA_ARGS__); \ } while (0) -#define hid_printk(level, hid, fmt, arg...) \ - dev_printk(level, &(hid)->dev, fmt, ##arg) -#define hid_emerg(hid, fmt, arg...) \ - dev_emerg(&(hid)->dev, fmt, ##arg) -#define hid_crit(hid, fmt, arg...) \ - dev_crit(&(hid)->dev, fmt, ##arg) -#define hid_alert(hid, fmt, arg...) \ - dev_alert(&(hid)->dev, fmt, ##arg) -#define hid_err(hid, fmt, arg...) \ - dev_err(&(hid)->dev, fmt, ##arg) -#define hid_notice(hid, fmt, arg...) \ - dev_notice(&(hid)->dev, fmt, ##arg) -#define hid_warn(hid, fmt, arg...) \ - dev_warn(&(hid)->dev, fmt, ##arg) -#define hid_info(hid, fmt, arg...) \ - dev_info(&(hid)->dev, fmt, ##arg) -#define hid_dbg(hid, fmt, arg...) \ - dev_dbg(&(hid)->dev, fmt, ##arg) +#define hid_err(hid, fmt, ...) \ + dev_err(&(hid)->dev, fmt, ##__VA_ARGS__) +#define hid_notice(hid, fmt, ...) \ + dev_notice(&(hid)->dev, fmt, ##__VA_ARGS__) +#define hid_warn(hid, fmt, ...) \ + dev_warn(&(hid)->dev, fmt, ##__VA_ARGS__) +#define hid_info(hid, fmt, ...) \ + dev_info(&(hid)->dev, fmt, ##__VA_ARGS__) +#define hid_dbg(hid, fmt, ...) \ + dev_dbg(&(hid)->dev, fmt, ##__VA_ARGS__) #endif -- 2.21.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/3] HID: core: Add printk_once variants to hid_warn() etc [not found] <20190812152022.27963-1-stillcompiling@gmail.com> 2019-08-12 15:20 ` [PATCH v3 1/3] HID: core: reformat and reduce hid_printk macros stillcompiling @ 2019-08-12 15:20 ` stillcompiling 2019-08-12 15:20 ` [PATCH v3 3/3] HID: core: fix dmesg flooding if report field larger than 32bit stillcompiling 2 siblings, 0 replies; 6+ messages in thread From: stillcompiling @ 2019-08-12 15:20 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires, open list:HID CORE LAYER, open list Cc: Joe Perches, Joshua Clayton From: Joshua Clayton <stillcompiling@gmail.com> hid_warn_once() is needed. Add the others as part of the block. Signed-off-by: Joshua Clayton <stillcompiling@gmail.com> diff --git a/include/linux/hid.h b/include/linux/hid.h index e6c7efdb0458..cd41f209043f 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -1171,4 +1171,15 @@ do { \ #define hid_dbg(hid, fmt, ...) \ dev_dbg(&(hid)->dev, fmt, ##__VA_ARGS__) +#define hid_err_once(hid, fmt, ...) \ + dev_err_once(&(hid)->dev, fmt, ##__VA_ARGS__) +#define hid_notice_once(hid, fmt, ...) \ + dev_notice_once(&(hid)->dev, fmt, ##__VA_ARGS__) +#define hid_warn_once(hid, fmt, ...) \ + dev_warn_once(&(hid)->dev, fmt, ##__VA_ARGS__) +#define hid_info_once(hid, fmt, ...) \ + dev_info_once(&(hid)->dev, fmt, ##__VA_ARGS__) +#define hid_dbg_once(hid, fmt, ...) \ + dev_dbg_once(&(hid)->dev, fmt, ##__VA_ARGS__) + #endif -- 2.21.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 3/3] HID: core: fix dmesg flooding if report field larger than 32bit [not found] <20190812152022.27963-1-stillcompiling@gmail.com> 2019-08-12 15:20 ` [PATCH v3 1/3] HID: core: reformat and reduce hid_printk macros stillcompiling 2019-08-12 15:20 ` [PATCH v3 2/3] HID: core: Add printk_once variants to hid_warn() etc stillcompiling @ 2019-08-12 15:20 ` stillcompiling 2019-08-28 23:26 ` Joshua Clayton 2 siblings, 1 reply; 6+ messages in thread From: stillcompiling @ 2019-08-12 15:20 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires, open list:HID CORE LAYER, open list Cc: Joe Perches, Joshua Clayton From: Joshua Clayton <stillcompiling@gmail.com> Only warn once of oversize hid report value field On HP spectre x360 convertible the message: hid-sensor-hub 001F:8087:0AC2.0002: hid_field_extract() called with n (192) > 32! (kworker/1:2) is continually printed many times per second, crowding out all else. Protect dmesg by printing the warning only one time. The size of the hid report field data structure should probably be increased. The data structure is treated as a u32 in Linux, but an unlimited number of bits in the USB hid spec, so there is some rearchitecture needed now that devices are sending more than 32 bits. Signed-off-by: Joshua Clayton <stillcompiling@gmail.com> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 210b81a56e1a..3eaee2c37931 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1311,8 +1311,8 @@ u32 hid_field_extract(const struct hid_device *hid, u8 *report, unsigned offset, unsigned n) { if (n > 32) { - hid_warn(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n", - n, current->comm); + hid_warn_once(hid, "%s() called with n (%d) > 32! (%s)\n", + __func__, n, current->comm); n = 32; } -- 2.21.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 3/3] HID: core: fix dmesg flooding if report field larger than 32bit 2019-08-12 15:20 ` [PATCH v3 3/3] HID: core: fix dmesg flooding if report field larger than 32bit stillcompiling @ 2019-08-28 23:26 ` Joshua Clayton 2019-09-18 15:35 ` Benjamin Tissoires 0 siblings, 1 reply; 6+ messages in thread From: Joshua Clayton @ 2019-08-28 23:26 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires, open list:HID CORE LAYER, open list Cc: Joe Perches ping? I'd love to see this get in. with distro kernel I have effectively no dmesg due to this issue On Mon, Aug 12, 2019 at 9:20 AM <stillcompiling@gmail.com> wrote: > > From: Joshua Clayton <stillcompiling@gmail.com> > > Only warn once of oversize hid report value field > > On HP spectre x360 convertible the message: > hid-sensor-hub 001F:8087:0AC2.0002: hid_field_extract() called with n (192) > 32! (kworker/1:2) > is continually printed many times per second, crowding out all else. > Protect dmesg by printing the warning only one time. > > The size of the hid report field data structure should probably be increased. > The data structure is treated as a u32 in Linux, but an unlimited number > of bits in the USB hid spec, so there is some rearchitecture needed now that > devices are sending more than 32 bits. > > Signed-off-by: Joshua Clayton <stillcompiling@gmail.com> > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 210b81a56e1a..3eaee2c37931 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1311,8 +1311,8 @@ u32 hid_field_extract(const struct hid_device *hid, u8 *report, > unsigned offset, unsigned n) > { > if (n > 32) { > - hid_warn(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n", > - n, current->comm); > + hid_warn_once(hid, "%s() called with n (%d) > 32! (%s)\n", > + __func__, n, current->comm); > n = 32; > } > > -- > 2.21.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 3/3] HID: core: fix dmesg flooding if report field larger than 32bit 2019-08-28 23:26 ` Joshua Clayton @ 2019-09-18 15:35 ` Benjamin Tissoires 2019-09-19 3:28 ` Joshua Clayton 0 siblings, 1 reply; 6+ messages in thread From: Benjamin Tissoires @ 2019-09-18 15:35 UTC (permalink / raw) To: Joshua Clayton Cc: Jiri Kosina, open list:HID CORE LAYER, open list, Joe Perches On Thu, Aug 29, 2019 at 1:26 AM Joshua Clayton <stillcompiling@gmail.com> wrote: > > ping? > I'd love to see this get in. > with distro kernel I have effectively no dmesg due to this issue Apologies for the delay. I really thought we should find a better way of fixing this, until I got a laptop affected by it. This series is a must have. Applied to for-5.4/core Cheers, Benjamin > > On Mon, Aug 12, 2019 at 9:20 AM <stillcompiling@gmail.com> wrote: > > > > From: Joshua Clayton <stillcompiling@gmail.com> > > > > Only warn once of oversize hid report value field > > > > On HP spectre x360 convertible the message: > > hid-sensor-hub 001F:8087:0AC2.0002: hid_field_extract() called with n (192) > 32! (kworker/1:2) > > is continually printed many times per second, crowding out all else. > > Protect dmesg by printing the warning only one time. > > > > The size of the hid report field data structure should probably be increased. > > The data structure is treated as a u32 in Linux, but an unlimited number > > of bits in the USB hid spec, so there is some rearchitecture needed now that > > devices are sending more than 32 bits. > > > > Signed-off-by: Joshua Clayton <stillcompiling@gmail.com> > > > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > > index 210b81a56e1a..3eaee2c37931 100644 > > --- a/drivers/hid/hid-core.c > > +++ b/drivers/hid/hid-core.c > > @@ -1311,8 +1311,8 @@ u32 hid_field_extract(const struct hid_device *hid, u8 *report, > > unsigned offset, unsigned n) > > { > > if (n > 32) { > > - hid_warn(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n", > > - n, current->comm); > > + hid_warn_once(hid, "%s() called with n (%d) > 32! (%s)\n", > > + __func__, n, current->comm); > > n = 32; > > } > > > > -- > > 2.21.0 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 3/3] HID: core: fix dmesg flooding if report field larger than 32bit 2019-09-18 15:35 ` Benjamin Tissoires @ 2019-09-19 3:28 ` Joshua Clayton 0 siblings, 0 replies; 6+ messages in thread From: Joshua Clayton @ 2019-09-19 3:28 UTC (permalink / raw) To: Benjamin Tissoires Cc: Jiri Kosina, open list:HID CORE LAYER, open list, Joe Perches Thanks! It means a lot to have this accepted. I actually started working on it, thinking "how hard can it be to increase the size of a data structure"? It only has to be forward compatible anyway. My gut feeling is the existing code is working way too hard to do what should be a memcpy, and the impulse to "fix" it is strong, despite my absolute lack of usb-hid experience. But the history of this little bit of code is already fraught with complaints about big endian breakage. I'm tempted to make it much simpler for size>32 bits (fix it only for future users), or just way simpler for little endian, But what do I know about usb and big endian? I sure don't have the equipment to test it. And I worry a little I might be forgetting some oddball non-byte-aligned data structure, which the spec would theoretically allow. Perhaps I'll have to time and courage to take another stab. ~Joshua On Wed, Sep 18, 2019 at 11:35 AM Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > > On Thu, Aug 29, 2019 at 1:26 AM Joshua Clayton <stillcompiling@gmail.com> wrote: > > > > ping? > > I'd love to see this get in. > > with distro kernel I have effectively no dmesg due to this issue > > Apologies for the delay. > > I really thought we should find a better way of fixing this, until I > got a laptop affected by it. This series is a must have. > > Applied to for-5.4/core > > Cheers, > Benjamin > > > > > On Mon, Aug 12, 2019 at 9:20 AM <stillcompiling@gmail.com> wrote: > > > > > > From: Joshua Clayton <stillcompiling@gmail.com> > > > > > > Only warn once of oversize hid report value field > > > > > > On HP spectre x360 convertible the message: > > > hid-sensor-hub 001F:8087:0AC2.0002: hid_field_extract() called with n (192) > 32! (kworker/1:2) > > > is continually printed many times per second, crowding out all else. > > > Protect dmesg by printing the warning only one time. > > > > > > The size of the hid report field data structure should probably be increased. > > > The data structure is treated as a u32 in Linux, but an unlimited number > > > of bits in the USB hid spec, so there is some rearchitecture needed now that > > > devices are sending more than 32 bits. > > > > > > Signed-off-by: Joshua Clayton <stillcompiling@gmail.com> > > > > > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > > > index 210b81a56e1a..3eaee2c37931 100644 > > > --- a/drivers/hid/hid-core.c > > > +++ b/drivers/hid/hid-core.c > > > @@ -1311,8 +1311,8 @@ u32 hid_field_extract(const struct hid_device *hid, u8 *report, > > > unsigned offset, unsigned n) > > > { > > > if (n > 32) { > > > - hid_warn(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n", > > > - n, current->comm); > > > + hid_warn_once(hid, "%s() called with n (%d) > 32! (%s)\n", > > > + __func__, n, current->comm); > > > n = 32; > > > } > > > > > > -- > > > 2.21.0 > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-19 3:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20190812152022.27963-1-stillcompiling@gmail.com>
2019-08-12 15:20 ` [PATCH v3 1/3] HID: core: reformat and reduce hid_printk macros stillcompiling
2019-08-12 15:20 ` [PATCH v3 2/3] HID: core: Add printk_once variants to hid_warn() etc stillcompiling
2019-08-12 15:20 ` [PATCH v3 3/3] HID: core: fix dmesg flooding if report field larger than 32bit stillcompiling
2019-08-28 23:26 ` Joshua Clayton
2019-09-18 15:35 ` Benjamin Tissoires
2019-09-19 3:28 ` Joshua Clayton
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).