From: Joshua Clayton <[email protected]>
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 other kernel logs
Protect dmesg by printing the warning only one time.
Signed-off-by: Joshua Clayton <[email protected]>
---
drivers/hid/hid-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 210b81a56e1a..7afd0422b280 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1311,7 +1311,7 @@ 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",
+ hid_warn_once(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n",
n, current->comm);
n = 32;
}
--
2.21.0
On Mon, 2019-07-22 at 10:36 -0600, [email protected] wrote:
> 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 other kernel logs
> Protect dmesg by printing the warning only one time.
[]
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
[]
> @@ -1311,7 +1311,7 @@ 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",
> + hid_warn_once(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n",
> n, current->comm);
> n = 32;
> }
Is this papering over an actual defect somewhere else?
Trivially, this could use "%s: ...", __func__, ...
On Mon, Jul 22, 2019 at 11:23 AM Joe Perches <[email protected]> wrote:
>
> On Mon, 2019-07-22 at 10:36 -0600, [email protected] wrote:
> > 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 other kernel logs
> > Protect dmesg by printing the warning only one time.
> []
> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> []
> > @@ -1311,7 +1311,7 @@ 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",
> > + hid_warn_once(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n",
> > n, current->comm);
> > n = 32;
> > }
>
> Is this papering over an actual defect somewhere else?
Sort of.
It doesn't correct the underlying issue, but I think this should go in
even along with the real fix.
The dmesg spamming has become a more serious problem for me than the
underlying issue.
Someone had a patch rejected that completely suppressed the message.
From my limited understanding, the hid spec allows an unlimited size
for an hid report , but the kernel only allocated 32 bits, which was
more than anything used at that time. The 32 bit version is doing some
bit shifting and possibly endian correction with the 32 bit field, so
I was not comfortable just extending it to 192 or 256 bits without a
little more understanding.
> Trivially, this could use "%s: ...", __func__, ...
True. I can make that change.
>
From: Joshua Clayton <[email protected]>
Reformat hid_printk macros to use standard __VA_ARGS__ syntax
Remove hid_printk(), hid_emerg(), hid_crit(), and hid_alert().
Per Joe Perches these unused and likely never to be used.
Signed-off-by: Joshua Clayton <[email protected]>
---
include/linux/hid.h | 32 ++++++++++++--------------------
1 file changed, 12 insertions(+), 20 deletions(-)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index d770ab1a0479..b5e73331100e 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
From: Joshua Clayton <[email protected]>
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.
Signed-off-by: Joshua Clayton <[email protected]>
---
drivers/hid/hid-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 210b81a56e1a..0cb53dddf341 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
On Mon, 2019-07-22 at 15:26 -0600, [email protected] wrote:
> From: Joshua Clayton <[email protected]>
Thanks Joshua
> 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.
[]
> diff --git 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);
All the other bits are fine, but this line is oddly written
with unusual spacing around 'n'.
Normally it'd be something like:
hid_warn_once(hid, "%s: called with n (%d) > 32! (%s)\n",
__func__, n, current->comm);
On Mon, Jul 22, 2019 at 3:30 PM Joe Perches <[email protected]> wrote:
>
> On Mon, 2019-07-22 at 15:26 -0600, [email protected] wrote:
> > From: Joshua Clayton <[email protected]>
>
> Thanks Joshua
>
> > 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.
> []
> > diff --git 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);
>
> All the other bits are fine, but this line is oddly written
> with unusual spacing around 'n'.
>
> Normally it'd be something like:
>
> hid_warn_once(hid, "%s: called with n (%d) > 32! (%s)\n",
> __func__, n, current->comm);
Gah!
Not only that but I missed a semicolon in patch 1. Will fix, (compile)
and send v3 pdq.
Sorry about the extra noise.
>
>
From: Joshua Clayton <[email protected]>
Reformat hid_printk macros to use standard __VA_ARGS__ syntax
Remove hid_printk(), hid_emerg(), hid_crit(), and hid_alert().
Per Joe Perches these unused and likely never to be used.
Signed-off-by: Joshua Clayton <[email protected]>
---
include/linux/hid.h | 32 ++++++++++++--------------------
1 file changed, 12 insertions(+), 20 deletions(-)
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
From: Joshua Clayton <[email protected]>
hid_warn_once() is needed, add the others as part of the block
Signed-off-by: Joshua Clayton <[email protected]>
---
include/linux/hid.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index b5e73331100e..306dde3760a4 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
From: Joshua Clayton <[email protected]>
hid_warn_once() is needed, add the others as part of the block
Signed-off-by: Joshua Clayton <[email protected]>
---
include/linux/hid.h | 11 +++++++++++
1 file changed, 11 insertions(+)
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