Re: [PATCH libinput] evdev: Log evdev event queue overflows
On Mon, Oct 27, 2014 at 09:26:39AM -0500, Derek Foreman wrote:
Log a message when the kernel event queue overflows and events are dropped.
After 10 messages logging stops to avoid flooding the logs if the condition
is persistent.
---
src/evdev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/evdev.c b/src/evdev.c
index 1b4ce10..c786537 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -912,6 +912,7 @@ evdev_sync_device(struct evdev_device *device)
static void
evdev_device_dispatch(void *data)
{
+ static int overflows = 0;
Does libinput allow static variables? Usually when there's a library
context, it should be possible to use the library from multiple threads,
if they use different contexts.
So maybe this counter should be inside `device` instead of static?
Ran
struct evdev_device *device = data;
struct libinput *libinput = device-base.seat-libinput;
struct input_event ev;
@@ -924,6 +925,15 @@ evdev_device_dispatch(void *data)
rc = libevdev_next_event(device-evdev,
LIBEVDEV_READ_FLAG_NORMAL, ev);
if (rc == LIBEVDEV_READ_STATUS_SYNC) {
+ if (overflows 10) {
+ overflows++;
+ log_info(libinput, Kernel evdev event queue
+ overflow for %s\n, device-devname);
+ if (overflows == 10)
+ log_info(libinput, No longer logging
+ evdev event queue
overflows\n);
+ }
+
/* send one more sync event so we handle all
currently pending events before we sync up
to the current state */
--
2.1.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] evdev: Log evdev event queue overflows
On 27/10/14 07:11 PM, Peter Hutterer wrote:
On Mon, Oct 27, 2014 at 09:33:45AM -0500, Derek Foreman wrote:
A couple of questions on this one:
Is it ok to limit logging to 10 messages like this?
IMO yes.
Should I be doing that on a per device basis instead of globally?
you are doing it per-device here, I'm not sure what you mean with the
question, sorry.
I'm doing it what I'd consider globally - 10 errors will disable the
message whether the errors are from the same or different devices.
Per device was intended to mean: adding the variable to the device
structure so each device (or the same device if it was
unplugged/re-plugged) would have its own independent 10 errors.
(I'm totally unattached to the specifics of the log text, I believe X
says something clever about how it's not the X server's fault to avoid
silly bug reports...)
that's a different error message, caused when the server-internal EQ
overflows. and the reason for the message is because the backtrace that X
prints then shows the input code as culprit, when the real issue is 5-6
stack frames up the stack. this wouldn't be necessary here, though we could
theoretically run into the same issue: if rendering delays reading from the
input fds for too long, we can trigger SYN_DROPPED.
Ah, ok.
What I'm seeing is what you describe at the end of the paragraph: the
(pixman) renderer taking enough time that we trigger SYN_DROPPED.
Cheers,
Peter
On 27/10/14 09:26 AM, Derek Foreman wrote:
Log a message when the kernel event queue overflows and events are dropped.
After 10 messages logging stops to avoid flooding the logs if the condition
is persistent.
---
src/evdev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/evdev.c b/src/evdev.c
index 1b4ce10..c786537 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -912,6 +912,7 @@ evdev_sync_device(struct evdev_device *device)
static void
evdev_device_dispatch(void *data)
{
+ static int overflows = 0;
struct evdev_device *device = data;
struct libinput *libinput = device-base.seat-libinput;
struct input_event ev;
@@ -924,6 +925,15 @@ evdev_device_dispatch(void *data)
rc = libevdev_next_event(device-evdev,
LIBEVDEV_READ_FLAG_NORMAL, ev);
if (rc == LIBEVDEV_READ_STATUS_SYNC) {
+ if (overflows 10) {
+ overflows++;
+ log_info(libinput, Kernel evdev event queue
+overflow for %s\n, device-devname);
+ if (overflows == 10)
+ log_info(libinput, No longer logging
+evdev event queue
overflows\n);
+ }
+
/* send one more sync event so we handle all
currently pending events before we sync up
to the current state */
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] evdev: Log evdev event queue overflows
On 28/10/14 03:20 AM, Ran Benita wrote:
On Mon, Oct 27, 2014 at 09:26:39AM -0500, Derek Foreman wrote:
Log a message when the kernel event queue overflows and events are dropped.
After 10 messages logging stops to avoid flooding the logs if the condition
is persistent.
---
src/evdev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/evdev.c b/src/evdev.c
index 1b4ce10..c786537 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -912,6 +912,7 @@ evdev_sync_device(struct evdev_device *device)
static void
evdev_device_dispatch(void *data)
{
+static int overflows = 0;
Does libinput allow static variables? Usually when there's a library
context, it should be possible to use the library from multiple threads,
if they use different contexts.
So maybe this counter should be inside `device` instead of static?
I'm not sure libinput is completely thread safe, but that's an easy
change to make.
I'd like to put it in struct libinput to preserve the current behaviour
- but that puts an evdev specific wart in an otherwise generic structure.
Any objections?
Ran
struct evdev_device *device = data;
struct libinput *libinput = device-base.seat-libinput;
struct input_event ev;
@@ -924,6 +925,15 @@ evdev_device_dispatch(void *data)
rc = libevdev_next_event(device-evdev,
LIBEVDEV_READ_FLAG_NORMAL, ev);
if (rc == LIBEVDEV_READ_STATUS_SYNC) {
+if (overflows 10) {
+overflows++;
+log_info(libinput, Kernel evdev event queue
+ overflow for %s\n, device-devname);
+if (overflows == 10)
+log_info(libinput, No longer logging
+ evdev event queue
overflows\n);
+}
+
/* send one more sync event so we handle all
currently pending events before we sync up
to the current state */
--
2.1.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] evdev: Log evdev event queue overflows
I can't imagine any way this static variable will mess up with multiple
threads. At worst not exactly 10 messages will be printed but that seems
really harmless. Every processor writes to memory a number larger than
it read, so eventually every processor will see a number = 10 even if
there was absolutely no synchronization.
On 10/28/2014 12:18 PM, Derek Foreman wrote:
On 28/10/14 03:20 AM, Ran Benita wrote:
On Mon, Oct 27, 2014 at 09:26:39AM -0500, Derek Foreman wrote:
Log a message when the kernel event queue overflows and events are dropped.
After 10 messages logging stops to avoid flooding the logs if the condition
is persistent.
---
src/evdev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/evdev.c b/src/evdev.c
index 1b4ce10..c786537 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -912,6 +912,7 @@ evdev_sync_device(struct evdev_device *device)
static void
evdev_device_dispatch(void *data)
{
+ static int overflows = 0;
Does libinput allow static variables? Usually when there's a library
context, it should be possible to use the library from multiple threads,
if they use different contexts.
So maybe this counter should be inside `device` instead of static?
I'm not sure libinput is completely thread safe, but that's an easy
change to make.
I'd like to put it in struct libinput to preserve the current behaviour
- but that puts an evdev specific wart in an otherwise generic structure.
Any objections?
Ran
struct evdev_device *device = data;
struct libinput *libinput = device-base.seat-libinput;
struct input_event ev;
@@ -924,6 +925,15 @@ evdev_device_dispatch(void *data)
rc = libevdev_next_event(device-evdev,
LIBEVDEV_READ_FLAG_NORMAL, ev);
if (rc == LIBEVDEV_READ_STATUS_SYNC) {
+ if (overflows 10) {
+ overflows++;
+ log_info(libinput, Kernel evdev event queue
+overflow for %s\n, device-devname);
+ if (overflows == 10)
+ log_info(libinput, No longer logging
+evdev event queue
overflows\n);
+ }
+
/* send one more sync event so we handle all
currently pending events before we sync up
to the current state */
--
2.1.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] evdev: Log evdev event queue overflows
On Tue, Oct 28, 2014 at 02:18:25PM -0500, Derek Foreman wrote:
On 28/10/14 03:20 AM, Ran Benita wrote:
On Mon, Oct 27, 2014 at 09:26:39AM -0500, Derek Foreman wrote:
Log a message when the kernel event queue overflows and events are dropped.
After 10 messages logging stops to avoid flooding the logs if the condition
is persistent.
---
src/evdev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/evdev.c b/src/evdev.c
index 1b4ce10..c786537 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -912,6 +912,7 @@ evdev_sync_device(struct evdev_device *device)
static void
evdev_device_dispatch(void *data)
{
+ static int overflows = 0;
Does libinput allow static variables? Usually when there's a library
context, it should be possible to use the library from multiple threads,
if they use different contexts.
So maybe this counter should be inside `device` instead of static?
I'm not sure libinput is completely thread safe, but that's an easy
change to make.
I'd like to put it in struct libinput to preserve the current behaviour
- but that puts an evdev specific wart in an otherwise generic structure.
Any objections?
just put it into struct evdev_device, that's where it belongs best. means
you get the counter per device, but that's not a big deal IMO.
Cheers,
Peter
struct evdev_device *device = data;
struct libinput *libinput = device-base.seat-libinput;
struct input_event ev;
@@ -924,6 +925,15 @@ evdev_device_dispatch(void *data)
rc = libevdev_next_event(device-evdev,
LIBEVDEV_READ_FLAG_NORMAL, ev);
if (rc == LIBEVDEV_READ_STATUS_SYNC) {
+ if (overflows 10) {
+ overflows++;
+ log_info(libinput, Kernel evdev event queue
+ overflow for %s\n, device-devname);
+ if (overflows == 10)
+ log_info(libinput, No longer logging
+ evdev event queue
overflows\n);
+ }
+
/* send one more sync event so we handle all
currently pending events before we sync up
to the current state */
--
2.1.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] evdev: Log evdev event queue overflows
On Tue, Oct 28, 2014 at 09:32:20AM -0500, Derek Foreman wrote:
On 27/10/14 07:11 PM, Peter Hutterer wrote:
On Mon, Oct 27, 2014 at 09:33:45AM -0500, Derek Foreman wrote:
A couple of questions on this one:
Is it ok to limit logging to 10 messages like this?
IMO yes.
Should I be doing that on a per device basis instead of globally?
you are doing it per-device here, I'm not sure what you mean with the
question, sorry.
I'm doing it what I'd consider globally - 10 errors will disable the
message whether the errors are from the same or different devices.
Per device was intended to mean: adding the variable to the device
structure so each device (or the same device if it was
unplugged/re-plugged) would have its own independent 10 errors.
see my comment in the other email, I think that's fine. In most cases people
use one or two devices only anyway.
(I'm totally unattached to the specifics of the log text, I believe X
says something clever about how it's not the X server's fault to avoid
silly bug reports...)
that's a different error message, caused when the server-internal EQ
overflows. and the reason for the message is because the backtrace that X
prints then shows the input code as culprit, when the real issue is 5-6
stack frames up the stack. this wouldn't be necessary here, though we could
theoretically run into the same issue: if rendering delays reading from the
input fds for too long, we can trigger SYN_DROPPED.
Ah, ok.
What I'm seeing is what you describe at the end of the paragraph: the
(pixman) renderer taking enough time that we trigger SYN_DROPPED.
short of moving input to its own thread, I don't think there's a good
solution to that. X avoids it by using SIGIO which works but is far from
ideal. And libinput is not designed to run in the signal handler anyway.
Cheers,
Peter
On 27/10/14 09:26 AM, Derek Foreman wrote:
Log a message when the kernel event queue overflows and events are
dropped.
After 10 messages logging stops to avoid flooding the logs if the
condition
is persistent.
---
src/evdev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/evdev.c b/src/evdev.c
index 1b4ce10..c786537 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -912,6 +912,7 @@ evdev_sync_device(struct evdev_device *device)
static void
evdev_device_dispatch(void *data)
{
+ static int overflows = 0;
struct evdev_device *device = data;
struct libinput *libinput = device-base.seat-libinput;
struct input_event ev;
@@ -924,6 +925,15 @@ evdev_device_dispatch(void *data)
rc = libevdev_next_event(device-evdev,
LIBEVDEV_READ_FLAG_NORMAL, ev);
if (rc == LIBEVDEV_READ_STATUS_SYNC) {
+ if (overflows 10) {
+ overflows++;
+ log_info(libinput, Kernel evdev event queue
+ overflow for %s\n, device-devname);
+ if (overflows == 10)
+ log_info(libinput, No longer logging
+ evdev event queue
overflows\n);
+ }
+
/* send one more sync event so we handle all
currently pending events before we sync up
to the current state */
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] evdev: Log evdev event queue overflows
A couple of questions on this one:
Is it ok to limit logging to 10 messages like this?
Should I be doing that on a per device basis instead of globally?
(I'm totally unattached to the specifics of the log text, I believe X
says something clever about how it's not the X server's fault to avoid
silly bug reports...)
Thanks
On 27/10/14 09:26 AM, Derek Foreman wrote:
Log a message when the kernel event queue overflows and events are dropped.
After 10 messages logging stops to avoid flooding the logs if the condition
is persistent.
---
src/evdev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/evdev.c b/src/evdev.c
index 1b4ce10..c786537 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -912,6 +912,7 @@ evdev_sync_device(struct evdev_device *device)
static void
evdev_device_dispatch(void *data)
{
+ static int overflows = 0;
struct evdev_device *device = data;
struct libinput *libinput = device-base.seat-libinput;
struct input_event ev;
@@ -924,6 +925,15 @@ evdev_device_dispatch(void *data)
rc = libevdev_next_event(device-evdev,
LIBEVDEV_READ_FLAG_NORMAL, ev);
if (rc == LIBEVDEV_READ_STATUS_SYNC) {
+ if (overflows 10) {
+ overflows++;
+ log_info(libinput, Kernel evdev event queue
+ overflow for %s\n, device-devname);
+ if (overflows == 10)
+ log_info(libinput, No longer logging
+ evdev event queue
overflows\n);
+ }
+
/* send one more sync event so we handle all
currently pending events before we sync up
to the current state */
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] evdev: Log evdev event queue overflows
On Mon, Oct 27, 2014 at 09:33:45AM -0500, Derek Foreman wrote:
A couple of questions on this one:
Is it ok to limit logging to 10 messages like this?
IMO yes.
Should I be doing that on a per device basis instead of globally?
you are doing it per-device here, I'm not sure what you mean with the
question, sorry.
(I'm totally unattached to the specifics of the log text, I believe X
says something clever about how it's not the X server's fault to avoid
silly bug reports...)
that's a different error message, caused when the server-internal EQ
overflows. and the reason for the message is because the backtrace that X
prints then shows the input code as culprit, when the real issue is 5-6
stack frames up the stack. this wouldn't be necessary here, though we could
theoretically run into the same issue: if rendering delays reading from the
input fds for too long, we can trigger SYN_DROPPED.
Cheers,
Peter
On 27/10/14 09:26 AM, Derek Foreman wrote:
Log a message when the kernel event queue overflows and events are dropped.
After 10 messages logging stops to avoid flooding the logs if the condition
is persistent.
---
src/evdev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/evdev.c b/src/evdev.c
index 1b4ce10..c786537 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -912,6 +912,7 @@ evdev_sync_device(struct evdev_device *device)
static void
evdev_device_dispatch(void *data)
{
+ static int overflows = 0;
struct evdev_device *device = data;
struct libinput *libinput = device-base.seat-libinput;
struct input_event ev;
@@ -924,6 +925,15 @@ evdev_device_dispatch(void *data)
rc = libevdev_next_event(device-evdev,
LIBEVDEV_READ_FLAG_NORMAL, ev);
if (rc == LIBEVDEV_READ_STATUS_SYNC) {
+ if (overflows 10) {
+ overflows++;
+ log_info(libinput, Kernel evdev event queue
+overflow for %s\n, device-devname);
+ if (overflows == 10)
+ log_info(libinput, No longer logging
+evdev event queue
overflows\n);
+ }
+
/* send one more sync event so we handle all
currently pending events before we sync up
to the current state */
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] evdev: Log evdev event queue overflows
On Mon, Oct 27, 2014 at 09:26:39AM -0500, Derek Foreman wrote:
Log a message when the kernel event queue overflows and events are dropped.
After 10 messages logging stops to avoid flooding the logs if the condition
is persistent.
---
src/evdev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/evdev.c b/src/evdev.c
index 1b4ce10..c786537 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -912,6 +912,7 @@ evdev_sync_device(struct evdev_device *device)
static void
evdev_device_dispatch(void *data)
{
+ static int overflows = 0;
tbh, I'd prefer some naming like syn_dropped_received or somesuch.
'overflow' has a lot of different meanings, syn_dropped means exactly one
thing.
same goes for the message, if you put SYN_DROPPED in there it's really easy
to google for it (e.g. libevdev has a page that details what happens).
Kernel evdev event queue overflow is harder to figure out, so we'd have to
explain it more often to peopl on the lists.
ack to the rest.
Cheers,
Peter
struct evdev_device *device = data;
struct libinput *libinput = device-base.seat-libinput;
struct input_event ev;
@@ -924,6 +925,15 @@ evdev_device_dispatch(void *data)
rc = libevdev_next_event(device-evdev,
LIBEVDEV_READ_FLAG_NORMAL, ev);
if (rc == LIBEVDEV_READ_STATUS_SYNC) {
+ if (overflows 10) {
+ overflows++;
+ log_info(libinput, Kernel evdev event queue
+ overflow for %s\n, device-devname);
+ if (overflows == 10)
+ log_info(libinput, No longer logging
+ evdev event queue
overflows\n);
+ }
+
/* send one more sync event so we handle all
currently pending events before we sync up
to the current state */
--
2.1.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
