Hi, ma, 2025-02-10 kello 13:33 -0500, Luiz Augusto von Dentz kirjoitti: [clip] > > It's not possible to disable wakeups on POLLERR & POLLHUP: > > > > https://man.archlinux.org/man/poll.2.en > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/eventpoll.c?h=v6.13&id=ffd294d346d185b70e28b1a28abe367bbfe53c04#n2391 > > Ah, so that means POLLERR and POLLHUP are always enabled together? I think the exact statement is that both POLLERR and POLLHUP are always enabled, no way to disable kernel issuing wakeup on either condition. > > You could use EPOLLET to only wake up once per event -- but bluez would > > then still wakeup on the 7.5ms interval. Glib also doesn't use epoll(), > > so there's more parts of GSource to reimplement, need to poll from > > separate thread etc., so the implementation gets more complex than > > here. > > > > To get something better than the timer-polled version, I think you'd > > need to add some new flag to kernel for this. > > Yeah, well I guess that would be even harder if we need to a new flag > since that would mean we would need to update glib to understand it > and then bump the dependency of it, or we would have to reimplement > the whole IO handling to use the non-glib version, the glib way will > most likely make releases adding a hard dependency for distros and > reimplementing the whole IO will probably require a lot of work, that > said Id welcome if we could work in that direction otherwise we need > another way to track the disconnection of ISO and A2DP streams. Another alternative here is to go with socket option etc., which we tried so that works, but maybe needs buy-in from netdev people. > > > > + if (watch->tag) > > > > + cond = g_source_query_unix_fd(&watch->source, watch->tag); > > > > + else > > > > + cond = 0; > > > > + > > > > + if (cond == G_IO_ERR) { > > > > + int err, ret; > > > > + socklen_t len = sizeof(err); > > > > + > > > > + ret = getsockopt(fd, SOL_SOCKET, SO_ERROR, &err, &len); > > > > + if (ret == 0 && err == 0) { > > > > + g_source_remove_unix_fd(&watch->source, watch->tag); > > > > + watch->tag = NULL; > > > > + > > > > + /* io_err watches all wake up at the same time */ > > > > + if (!io_err_watch_wakeup) > > > > + io_err_watch_wakeup = g_get_monotonic_time() > > > > + + timeout; > > > > + > > > > + g_source_set_ready_time(&watch->source, > > > > + io_err_watch_wakeup); > > > > + return TRUE; > > > > + } > > > > + } > > > > + > > > > + if (g_source_get_ready_time(&watch->source) != -1) { > > > > + g_assert(!watch->tag); > > > > + io_err_watch_wakeup = 0; > > > > + watch->tag = g_source_add_unix_fd(&watch->source, fd, > > > > + watch->events); > > > > + g_source_set_ready_time(&watch->source, -1); > > > > + } > > > > + > > > > + cond &= watch->events; > > > > + > > > > + if (cond) > > > > + return func(watch->io, cond, user_data); > > > > + else > > > > + return TRUE; > > > > +} > > > > + > > > > +static void io_err_watch_finalize(GSource *source) > > > > +{ > > > > + struct io_err_watch *watch = (void *)source; > > > > + > > > > + if (watch->tag) > > > > + g_source_remove_unix_fd(&watch->source, watch->tag); > > > > + > > > > + g_io_channel_unref(watch->io); > > > > +} > > > > + > > > > +guint io_glib_add_err_watch_full(GIOChannel *io, gint priority, > > > > + GIOCondition events, > > > > + GIOFunc func, gpointer user_data, > > > > + GDestroyNotify notify) > > > > +{ > > > > + static GSourceFuncs source_funcs = { > > > > + .dispatch = io_err_watch_dispatch, > > > > + .finalize = io_err_watch_finalize, > > > > + }; > > > > + GSourceFunc callback = (void *)func; > > > > + struct io_err_watch *watch; > > > > + gint fd; > > > > + guint id; > > > > + > > > > + g_return_val_if_fail(!(events & (G_IO_IN | G_IO_OUT)), 0); > > > > + g_return_val_if_fail(events, 0); > > > > + g_return_val_if_fail(func, 0); > > > > + > > > > + fd = g_io_channel_unix_get_fd(io); > > > > + > > > > + watch = (void *)g_source_new(&source_funcs, > > > > + sizeof(struct io_err_watch)); > > > > + > > > > + watch->io = g_io_channel_ref(io); > > > > + watch->events = events; > > > > + watch->tag = g_source_add_unix_fd(&watch->source, fd, events); > > > > + > > > > + g_source_set_name((void *)watch, "io_glib_err_watch"); > > > > + g_source_set_callback(&watch->source, callback, user_data, notify); > > > > + > > > > + if (priority != G_PRIORITY_DEFAULT) > > > > + g_source_set_priority(&watch->source, priority); > > > > + > > > > + id = g_source_attach(&watch->source, NULL); > > > > + g_source_unref(&watch->source); > > > > + > > > > + return id; > > > > +} > > > > + > > > > +guint io_glib_add_err_watch(GIOChannel *io, GIOCondition events, GIOFunc func, > > > > + gpointer user_data) > > > > +{ > > > > + return io_glib_add_err_watch_full(io, G_PRIORITY_DEFAULT, events, > > > > + func, user_data, NULL); > > > > +} > > > > diff --git a/src/shared/io-glib.h b/src/shared/io-glib.h > > > > new file mode 100644 > > > > index 000000000..1db6fd468 > > > > --- /dev/null > > > > +++ b/src/shared/io-glib.h > > > > @@ -0,0 +1,20 @@ > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > +/* > > > > + * > > > > + * BlueZ - Bluetooth protocol stack for Linux > > > > + * > > > > + * Copyright (C) 2012-2014 Intel Corporation. All rights reserved. > > > > + * > > > > + * > > > > + */ > > > > + > > > > +#include <glib.h> > > > > + > > > > +guint io_glib_add_err_watch(GIOChannel *io, GIOCondition events, > > > > + GIOFunc func, gpointer user_data); > > > > +guint io_glib_add_err_watch_full(GIOChannel *io, gint priority, > > > > + GIOCondition events, GIOFunc func, > > > > + gpointer user_data, > > > > + GDestroyNotify notify); > > > > + > > > > > > Hmm, I think it is probably not a good idea to start using headers > > > like this, I'd consider just making it return 0 for non-glib. > > > > Ok. > > > > > > +bool io_set_use_err_watch(struct io *io, bool err_watch); > > > > -- > > > > 2.48.1 > > > > -- > > Pauli Virtanen > > >
