On Thu, 2020-10-15 at 10:12 -0700, Luiz Augusto von Dentz wrote: > 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_not > > ify, idev); > > If this is happening isn't there a idev->req already set and we are > overwriting it? > It was definitely causing a problem, but I can't remember exactly what occurred, I wrote the patch originally years ago! I think it leaked the watch.
