Hi Steven, On Thu, Oct 15, 2020 at 4:13 AM Steven Newbury <steve@xxxxxxxxxxxxxxx> wrote: > > There are a couple of issues with g_io_channel usage in bluez which > cause CPUs to spin on half-closed channels. > > This patch fixes bugs where bluetooth keyboards fail to work on initial > connection, and cause 100% cpu on disconnect. > > Also fix bug with similar symptoms triggered by some other HID devices > such as Sony PS3 BD Remotes. > > In the previous discussion on the kernel bugzilla below, it was > suggested to remove sec_watch, and I attached a follow-up patch to do > so, however that change causes problems with current bluez-5 releases > where a fd is used after being closed. > > See https://bugzilla.kernel.org/show_bug.cgi?id=204275 > > Signed-off-by: Steven Newbury <steve@xxxxxxxxxxxxxxx> > --- > attrib/interactive.c | 4 +++- > profiles/input/device.c | 3 ++- > 2 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/attrib/interactive.c b/attrib/interactive.c > index 9a7976d34..453ff064e 100644 > --- a/attrib/interactive.c > +++ b/attrib/interactive.c > @@ -64,6 +64,7 @@ static int opt_psm = 0; > static int opt_mtu = 0; > static int start; > static int end; > +static guint gsrc; > > static void cmd_help(int argcp, char **argvp); > > @@ -193,6 +194,7 @@ static void disconnect_io() > attrib = NULL; > opt_mtu = 0; > > + g_source_remove(gsrc); > g_io_channel_shutdown(iochannel, FALSE, NULL); > g_io_channel_unref(iochannel); > iochannel = NULL; > @@ -415,7 +417,7 @@ static void cmd_connect(int argcp, char **argvp) > error("%s\n", gerr->message); > g_error_free(gerr); > } else > - g_io_add_watch(iochannel, G_IO_HUP, channel_watcher, NULL); > + gsrc = g_io_add_watch(iochannel, G_IO_HUP, channel_watcher, NULL); > } I wouldn't bother with the fix above since the attrib part will be going away soon. > static void cmd_disconnect(int argcp, char **argvp) > diff --git a/profiles/input/device.c b/profiles/input/device.c > index a711ef527..9abf595f6 100644 > --- a/profiles/input/device.c > +++ b/profiles/input/device.c > @@ -982,7 +982,8 @@ static int hidp_add_connection(struct input_device *idev) > } > > idev->req = req; > - idev->sec_watch = g_io_add_watch(idev->intr_io, G_IO_OUT, > + if (!idev->sec_watch) > + idev->sec_watch = g_io_add_watch(idev->intr_io, G_IO_IN, > encrypt_notify, idev); If this is happening isn't there a idev->req already set and we are overwriting it? > return 0; > -- > 2.22.0 > > -- Luiz Augusto von Dentz
