Re: [systemd-devel] [PATCH 1/2] Add sd_event_loop_timeout to sd_event
Le 04/02/2015 18:20, Lennart Poettering a écrit : On Wed, 04.02.15 17:40, Didier Roche (didro...@ubuntu.com) wrote: Le 04/02/2015 17:10, Lennart Poettering a écrit : On Wed, 04.02.15 17:05, Didier Roche (didro...@ubuntu.com) wrote: Hey, I rewrote a version of this patch including the feedback on the list. As per IRC discussion, (and after giving up the busy loop for a rewrite with epool), I did rebase it again on sd_event. I'm only proposing there up for review the 2 first patches (without plymouth communication, cancel support, i18n, man pages and the service and socket) so that I don't have to rebase all other 10 patches on a moving ground. Tom just added support for installing timer events with a NULL callback, that trigger event loop exit. I kinda prefer that solution over a new call sd_event_loop() with timeout. sd_event_add_time(event, NULL, CLOCK_MONOTONIC, now(CLOCK_MONOTONIC) + 5 * USEC_PER_SEC, NULL, NULL); So, it means that I need to reset it after any received activity, is that ok? (as this will be really frequent as each clients in parallel can send a message each 50ms). The goal is to have a global "inactivity" timeout. I didn't see a way to cancel this event source though? Oh, I see, you actually want a real idle logic, not just a normal timer. So far, for daemons like timedated, localed and so on, we are using an idle logic that is implemented in bus_event_loop_with_idle() in src/libsystemd/sd-bus/bus-util.c. It does considerably more than what you need (since it contains all the magic to racefully do exit-on-idle for bus services so that no bus messages are lost). I think the best would be to take inspiration from that code, isolate there basic minimum out of it, without all the dbus logic, and then stick that in your C file. We can generalize such exit-on-idle logic one day, somewhere between sd-bus and sd-event, but that requires considerabe design work, so that we find a generic solution that works for you and also covers this dbus case without hacks. For now it's hence better if you just take inspiration from bus_event_loop_with_idle(), drop all the bus-specific bits, and stick it in your .c code... Making sense. Done and fixed. Thanks a lot Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] Add sd_event_loop_timeout to sd_event
On Wed, 04.02.15 17:40, Didier Roche (didro...@ubuntu.com) wrote: > Le 04/02/2015 17:10, Lennart Poettering a écrit : > >On Wed, 04.02.15 17:05, Didier Roche (didro...@ubuntu.com) wrote: > > > >>Hey, > >> > >>I rewrote a version of this patch including the feedback on the list. As per > >>IRC discussion, (and after giving up the busy loop for a rewrite with > >>epool), I did rebase it again on sd_event. > >> > >>I'm only proposing there up for review the 2 first patches (without plymouth > >>communication, cancel support, i18n, man pages and the service and socket) > >>so that I don't have to rebase all other 10 patches on a moving > >>ground. > >Tom just added support for installing timer events with a NULL > >callback, that trigger event loop exit. I kinda prefer that solution > >over a new call sd_event_loop() with timeout. > > > > sd_event_add_time(event, NULL, CLOCK_MONOTONIC, now(CLOCK_MONOTONIC) + > > 5 * USEC_PER_SEC, NULL, NULL); > > So, it means that I need to reset it after any received activity, is that > ok? (as this will be really frequent as each clients in parallel can send a > message each 50ms). The goal is to have a global "inactivity" timeout. > > I didn't see a way to cancel this event source though? Oh, I see, you actually want a real idle logic, not just a normal timer. So far, for daemons like timedated, localed and so on, we are using an idle logic that is implemented in bus_event_loop_with_idle() in src/libsystemd/sd-bus/bus-util.c. It does considerably more than what you need (since it contains all the magic to racefully do exit-on-idle for bus services so that no bus messages are lost). I think the best would be to take inspiration from that code, isolate there basic minimum out of it, without all the dbus logic, and then stick that in your C file. We can generalize such exit-on-idle logic one day, somewhere between sd-bus and sd-event, but that requires considerabe design work, so that we find a generic solution that works for you and also covers this dbus case without hacks. For now it's hence better if you just take inspiration from bus_event_loop_with_idle(), drop all the bus-specific bits, and stick it in your .c code... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] Add sd_event_loop_timeout to sd_event
Le 04/02/2015 17:10, Lennart Poettering a écrit : On Wed, 04.02.15 17:05, Didier Roche (didro...@ubuntu.com) wrote: Hey, I rewrote a version of this patch including the feedback on the list. As per IRC discussion, (and after giving up the busy loop for a rewrite with epool), I did rebase it again on sd_event. I'm only proposing there up for review the 2 first patches (without plymouth communication, cancel support, i18n, man pages and the service and socket) so that I don't have to rebase all other 10 patches on a moving ground. Tom just added support for installing timer events with a NULL callback, that trigger event loop exit. I kinda prefer that solution over a new call sd_event_loop() with timeout. sd_event_add_time(event, NULL, CLOCK_MONOTONIC, now(CLOCK_MONOTONIC) + 5 * USEC_PER_SEC, NULL, NULL); So, it means that I need to reset it after any received activity, is that ok? (as this will be really frequent as each clients in parallel can send a message each 50ms). The goal is to have a global "inactivity" timeout. I didn't see a way to cancel this event source though? Didier That line should be enough to mak an even loop exit after 5s... Lennart ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] Add sd_event_loop_timeout to sd_event
On Wed, 04.02.15 17:05, Didier Roche (didro...@ubuntu.com) wrote: > Hey, > > I rewrote a version of this patch including the feedback on the list. As per > IRC discussion, (and after giving up the busy loop for a rewrite with > epool), I did rebase it again on sd_event. > > I'm only proposing there up for review the 2 first patches (without plymouth > communication, cancel support, i18n, man pages and the service and socket) > so that I don't have to rebase all other 10 patches on a moving > ground. Tom just added support for installing timer events with a NULL callback, that trigger event loop exit. I kinda prefer that solution over a new call sd_event_loop() with timeout. sd_event_add_time(event, NULL, CLOCK_MONOTONIC, now(CLOCK_MONOTONIC) + 5 * USEC_PER_SEC, NULL, NULL); That line should be enough to mak an even loop exit after 5s... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/2] Add sd_event_loop_timeout to sd_event
Hey, I rewrote a version of this patch including the feedback on the list. As per IRC discussion, (and after giving up the busy loop for a rewrite with epool), I did rebase it again on sd_event. I'm only proposing there up for review the 2 first patches (without plymouth communication, cancel support, i18n, man pages and the service and socket) so that I don't have to rebase all other 10 patches on a moving ground. I'm opened to any feedback and fixes to do :) Cheers, Didier >From 52d3c92280ec6198a0566bd3a077c0fbb6990782 Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Wed, 4 Feb 2015 16:40:44 +0100 Subject: [PATCH 1/2] Add sd_event_loop_timeout to sd_event sd_event_loop_timeout adds a timeout parameter to sd_event_loop() to timeout the whole event loop after some time of inactivity. --- src/libsystemd/sd-event/sd-event.c | 10 +++--- src/systemd/sd-event.h | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index f9fa54d..8dd3e16 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -2496,7 +2496,7 @@ _public_ int sd_event_run(sd_event *e, uint64_t timeout) { return r; } -_public_ int sd_event_loop(sd_event *e) { +_public_ int sd_event_loop_timeout(sd_event *e, uint64_t timeout) { int r; assert_return(e, -EINVAL); @@ -2506,8 +2506,8 @@ _public_ int sd_event_loop(sd_event *e) { sd_event_ref(e); while (e->state != SD_EVENT_FINISHED) { -r = sd_event_run(e, (uint64_t) -1); -if (r < 0) +r = sd_event_run(e, timeout); +if (r < 0 || (r == 0 && timeout != (uint64_t) -1)) goto finish; } @@ -2518,6 +2518,10 @@ finish: return r; } +_public_ int sd_event_loop(sd_event *e) { +return sd_event_loop_timeout(e, (uint64_t) -1); +} + _public_ int sd_event_get_fd(sd_event *e) { assert_return(e, -EINVAL); diff --git a/src/systemd/sd-event.h b/src/systemd/sd-event.h index 25a10f9..53e2382 100644 --- a/src/systemd/sd-event.h +++ b/src/systemd/sd-event.h @@ -90,6 +90,7 @@ int sd_event_prepare(sd_event *e); int sd_event_wait(sd_event *e, uint64_t timeout); int sd_event_dispatch(sd_event *e); int sd_event_run(sd_event *e, uint64_t timeout); +int sd_event_loop_timeout(sd_event *e, uint64_t timeout); int sd_event_loop(sd_event *e); int sd_event_exit(sd_event *e, int code); -- 2.1.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel