| Message ID | 20190123085923.12524-4-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 4d470eb37fe31cc9899ee36e81134b35b02a36f3 |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On 23/01/2019 08:59, Laurent Pinchart wrote: > The new interrupt() function allows interrupting in-progress blocking > processEvents() calls. This is useful to stop running event loops. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/event_dispatcher.h | 2 ++ > src/libcamera/event_dispatcher.cpp | 10 ++++++ > src/libcamera/event_dispatcher_poll.cpp | 32 ++++++++++++++++++- > src/libcamera/include/event_dispatcher_poll.h | 3 ++ > 4 files changed, 46 insertions(+), 1 deletion(-) > > diff --git a/include/libcamera/event_dispatcher.h b/include/libcamera/event_dispatcher.h > index c20518c6866d..cb06bf208e12 100644 > --- a/include/libcamera/event_dispatcher.h > +++ b/include/libcamera/event_dispatcher.h > @@ -26,6 +26,8 @@ public: > virtual void unregisterTimer(Timer *timer) = 0; > > virtual void processEvents() = 0; > + > + virtual void interrupt() = 0; > }; > > } /* namespace libcamera */ > diff --git a/src/libcamera/event_dispatcher.cpp b/src/libcamera/event_dispatcher.cpp > index f7c40734095e..b82c59c3f5dc 100644 > --- a/src/libcamera/event_dispatcher.cpp > +++ b/src/libcamera/event_dispatcher.cpp > @@ -104,4 +104,14 @@ EventDispatcher::~EventDispatcher() > * it before returning. > */ > > +/** > + * \fn EventDispatcher::interrupt() > + * \brief Interrupt any running processEvents() call as soon as possible > + * > + * Calling this function interrupts any blocking processEvents() call in > + * progress. The processEvents() function will return as soon as possible, > + * after processing pending timers and events. If processEvents() isn't in > + * progress, it will be interrupted immediately the next time it gets called. > + */ > + > } /* namespace libcamera */ > diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp > index 6e0609c34ddc..a2674ab31135 100644 > --- a/src/libcamera/event_dispatcher_poll.cpp > +++ b/src/libcamera/event_dispatcher_poll.cpp > @@ -8,7 +8,10 @@ > #include <algorithm> > #include <iomanip> > #include <poll.h> > +#include <stdint.h> > #include <string.h> > +#include <sys/eventfd.h> > +#include <unistd.h> > > #include <libcamera/event_notifier.h> > #include <libcamera/timer.h> > @@ -43,10 +46,18 @@ static const char *notifierType(EventNotifier::Type type) > > EventDispatcherPoll::EventDispatcherPoll() > { > + /* > + * Create the event fd. Failures are fatal as we can't implement an > + * interruptible dispatcher without the fd. > + */ > + eventfd_ = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); Hrm ... I didn't know about eventfd. I'd always used a pair of pipes. Every day's a school day :) The man page says it's linux-specific - but I think we're OK with that :-) > + if (eventfd_ < 0) > + LOG(Event, Fatal) << "Unable to create eventfd"; > } > > EventDispatcherPoll::~EventDispatcherPoll() > { > + close(eventfd_); > } > > void EventDispatcherPoll::registerEventNotifier(EventNotifier *notifier) > @@ -123,11 +134,13 @@ void EventDispatcherPoll::processEvents() > > /* Create the pollfd array. */ > std::vector<struct pollfd> pollfds; > - pollfds.reserve(notifiers_.size()); > + pollfds.reserve(notifiers_.size() + 1); > > for (auto notifier : notifiers_) > pollfds.push_back({ notifier.first, notifier.second.events(), 0 }); > > + pollfds.push_back({ eventfd_, POLLIN, 0 }); > + > /* Wait for events and process notifiers and timers. */ > do { > ret = poll(&pollfds); > @@ -137,12 +150,20 @@ void EventDispatcherPoll::processEvents() > ret = -errno; > LOG(Event, Warning) << "poll() failed with " << strerror(-ret); > } else if (ret > 0) { > + processInterrupt(pollfds.back()); > + pollfds.pop_back(); > processNotifiers(pollfds); > } > > processTimers(); > } > > +void EventDispatcherPoll::interrupt() > +{ > + uint64_t value = 1; > + write(eventfd_, &value, sizeof(value)); > +} > + > short EventDispatcherPoll::EventNotifierSetPoll::events() const > { > short events = 0; > @@ -186,6 +207,15 @@ int EventDispatcherPoll::poll(std::vector<struct pollfd> *pollfds) > nextTimer ? &timeout : nullptr, nullptr); > } > > +void EventDispatcherPoll::processInterrupt(const struct pollfd &pfd) > +{ > + if (!pfd.revents & POLLIN) > + return; > + > + uint64_t value; > + read(eventfd_, &value, sizeof(value)); Should we check that the value read was expected? I guess this could be expanded upon to use an events bitfield or enum to actually pass meaningful messages too. I'm not sure what those event messages would be yet - so I don't think we need to implement without a use-case. -- Regards Kieran > +} > + > void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pollfds) > { > static const struct { > diff --git a/src/libcamera/include/event_dispatcher_poll.h b/src/libcamera/include/event_dispatcher_poll.h > index ac3efde082b4..1c0066c24dc8 100644 > --- a/src/libcamera/include/event_dispatcher_poll.h > +++ b/src/libcamera/include/event_dispatcher_poll.h > @@ -31,6 +31,7 @@ public: > void unregisterTimer(Timer *timer); > > void processEvents(); > + void interrupt(); > > private: > struct EventNotifierSetPoll { > @@ -40,8 +41,10 @@ private: > > std::map<int, EventNotifierSetPoll> notifiers_; > std::list<Timer *> timers_; > + int eventfd_; > > int poll(std::vector<struct pollfd> *pollfds); > + void processInterrupt(const struct pollfd &pfd); > void processNotifiers(const std::vector<struct pollfd> &pollfds); > void processTimers(); > }; >
Hi Kieran, On Wed, Jan 23, 2019 at 11:43:21AM +0000, Kieran Bingham wrote: > On 23/01/2019 08:59, Laurent Pinchart wrote: > > The new interrupt() function allows interrupting in-progress blocking > > processEvents() calls. This is useful to stop running event loops. > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/libcamera/event_dispatcher.h | 2 ++ > > src/libcamera/event_dispatcher.cpp | 10 ++++++ > > src/libcamera/event_dispatcher_poll.cpp | 32 ++++++++++++++++++- > > src/libcamera/include/event_dispatcher_poll.h | 3 ++ > > 4 files changed, 46 insertions(+), 1 deletion(-) > > > > diff --git a/include/libcamera/event_dispatcher.h b/include/libcamera/event_dispatcher.h > > index c20518c6866d..cb06bf208e12 100644 > > --- a/include/libcamera/event_dispatcher.h > > +++ b/include/libcamera/event_dispatcher.h > > @@ -26,6 +26,8 @@ public: > > virtual void unregisterTimer(Timer *timer) = 0; > > > > virtual void processEvents() = 0; > > + > > + virtual void interrupt() = 0; > > }; > > > > } /* namespace libcamera */ > > diff --git a/src/libcamera/event_dispatcher.cpp b/src/libcamera/event_dispatcher.cpp > > index f7c40734095e..b82c59c3f5dc 100644 > > --- a/src/libcamera/event_dispatcher.cpp > > +++ b/src/libcamera/event_dispatcher.cpp > > @@ -104,4 +104,14 @@ EventDispatcher::~EventDispatcher() > > * it before returning. > > */ > > > > +/** > > + * \fn EventDispatcher::interrupt() > > + * \brief Interrupt any running processEvents() call as soon as possible > > + * > > + * Calling this function interrupts any blocking processEvents() call in > > + * progress. The processEvents() function will return as soon as possible, > > + * after processing pending timers and events. If processEvents() isn't in > > + * progress, it will be interrupted immediately the next time it gets called. > > + */ > > + > > } /* namespace libcamera */ > > diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp > > index 6e0609c34ddc..a2674ab31135 100644 > > --- a/src/libcamera/event_dispatcher_poll.cpp > > +++ b/src/libcamera/event_dispatcher_poll.cpp > > @@ -8,7 +8,10 @@ > > #include <algorithm> > > #include <iomanip> > > #include <poll.h> > > +#include <stdint.h> > > #include <string.h> > > +#include <sys/eventfd.h> > > +#include <unistd.h> > > > > #include <libcamera/event_notifier.h> > > #include <libcamera/timer.h> > > @@ -43,10 +46,18 @@ static const char *notifierType(EventNotifier::Type type) > > > > EventDispatcherPoll::EventDispatcherPoll() > > { > > + /* > > + * Create the event fd. Failures are fatal as we can't implement an > > + * interruptible dispatcher without the fd. > > + */ > > + eventfd_ = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); > > Hrm ... I didn't know about eventfd. I'd always used a pair of pipes. > > Every day's a school day :) > > The man page says it's linux-specific - but I think we're OK with that :-) When we'll start supporting other operating systems we'll abstract this in a class :-) > > + if (eventfd_ < 0) > > + LOG(Event, Fatal) << "Unable to create eventfd"; > > } > > > > EventDispatcherPoll::~EventDispatcherPoll() > > { > > + close(eventfd_); > > } > > > > void EventDispatcherPoll::registerEventNotifier(EventNotifier *notifier) > > @@ -123,11 +134,13 @@ void EventDispatcherPoll::processEvents() > > > > /* Create the pollfd array. */ > > std::vector<struct pollfd> pollfds; > > - pollfds.reserve(notifiers_.size()); > > + pollfds.reserve(notifiers_.size() + 1); > > > > for (auto notifier : notifiers_) > > pollfds.push_back({ notifier.first, notifier.second.events(), 0 }); > > > > + pollfds.push_back({ eventfd_, POLLIN, 0 }); > > + > > /* Wait for events and process notifiers and timers. */ > > do { > > ret = poll(&pollfds); > > @@ -137,12 +150,20 @@ void EventDispatcherPoll::processEvents() > > ret = -errno; > > LOG(Event, Warning) << "poll() failed with " << strerror(-ret); > > } else if (ret > 0) { > > + processInterrupt(pollfds.back()); > > + pollfds.pop_back(); > > processNotifiers(pollfds); > > } > > > > processTimers(); > > } > > > > +void EventDispatcherPoll::interrupt() > > +{ > > + uint64_t value = 1; > > + write(eventfd_, &value, sizeof(value)); > > +} > > + > > short EventDispatcherPoll::EventNotifierSetPoll::events() const > > { > > short events = 0; > > @@ -186,6 +207,15 @@ int EventDispatcherPoll::poll(std::vector<struct pollfd> *pollfds) > > nextTimer ? &timeout : nullptr, nullptr); > > } > > > > +void EventDispatcherPoll::processInterrupt(const struct pollfd &pfd) > > +{ > > + if (!pfd.revents & POLLIN) > > + return; > > + > > + uint64_t value; > > + read(eventfd_, &value, sizeof(value)); > > Should we check that the value read was expected? > > I guess this could be expanded upon to use an events bitfield or enum to > actually pass meaningful messages too. > > I'm not sure what those event messages would be yet - so I don't think > we need to implement without a use-case. eventfd creates an internal counter, initialized to 0. The write() call adds the value of the data to the internal counter, and the read() call returns the value of the counter and resets if to 0. The poll() call will block until the counter is non-zero. I don't think there's anything meaningful we could do we the read value here. > > +} > > + > > void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pollfds) > > { > > static const struct { > > diff --git a/src/libcamera/include/event_dispatcher_poll.h b/src/libcamera/include/event_dispatcher_poll.h > > index ac3efde082b4..1c0066c24dc8 100644 > > --- a/src/libcamera/include/event_dispatcher_poll.h > > +++ b/src/libcamera/include/event_dispatcher_poll.h > > @@ -31,6 +31,7 @@ public: > > void unregisterTimer(Timer *timer); > > > > void processEvents(); > > + void interrupt(); > > > > private: > > struct EventNotifierSetPoll { > > @@ -40,8 +41,10 @@ private: > > > > std::map<int, EventNotifierSetPoll> notifiers_; > > std::list<Timer *> timers_; > > + int eventfd_; > > > > int poll(std::vector<struct pollfd> *pollfds); > > + void processInterrupt(const struct pollfd &pfd); > > void processNotifiers(const std::vector<struct pollfd> &pollfds); > > void processTimers(); > > };
diff --git a/include/libcamera/event_dispatcher.h b/include/libcamera/event_dispatcher.h index c20518c6866d..cb06bf208e12 100644 --- a/include/libcamera/event_dispatcher.h +++ b/include/libcamera/event_dispatcher.h @@ -26,6 +26,8 @@ public: virtual void unregisterTimer(Timer *timer) = 0; virtual void processEvents() = 0; + + virtual void interrupt() = 0; }; } /* namespace libcamera */ diff --git a/src/libcamera/event_dispatcher.cpp b/src/libcamera/event_dispatcher.cpp index f7c40734095e..b82c59c3f5dc 100644 --- a/src/libcamera/event_dispatcher.cpp +++ b/src/libcamera/event_dispatcher.cpp @@ -104,4 +104,14 @@ EventDispatcher::~EventDispatcher() * it before returning. */ +/** + * \fn EventDispatcher::interrupt() + * \brief Interrupt any running processEvents() call as soon as possible + * + * Calling this function interrupts any blocking processEvents() call in + * progress. The processEvents() function will return as soon as possible, + * after processing pending timers and events. If processEvents() isn't in + * progress, it will be interrupted immediately the next time it gets called. + */ + } /* namespace libcamera */ diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp index 6e0609c34ddc..a2674ab31135 100644 --- a/src/libcamera/event_dispatcher_poll.cpp +++ b/src/libcamera/event_dispatcher_poll.cpp @@ -8,7 +8,10 @@ #include <algorithm> #include <iomanip> #include <poll.h> +#include <stdint.h> #include <string.h> +#include <sys/eventfd.h> +#include <unistd.h> #include <libcamera/event_notifier.h> #include <libcamera/timer.h> @@ -43,10 +46,18 @@ static const char *notifierType(EventNotifier::Type type) EventDispatcherPoll::EventDispatcherPoll() { + /* + * Create the event fd. Failures are fatal as we can't implement an + * interruptible dispatcher without the fd. + */ + eventfd_ = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); + if (eventfd_ < 0) + LOG(Event, Fatal) << "Unable to create eventfd"; } EventDispatcherPoll::~EventDispatcherPoll() { + close(eventfd_); } void EventDispatcherPoll::registerEventNotifier(EventNotifier *notifier) @@ -123,11 +134,13 @@ void EventDispatcherPoll::processEvents() /* Create the pollfd array. */ std::vector<struct pollfd> pollfds; - pollfds.reserve(notifiers_.size()); + pollfds.reserve(notifiers_.size() + 1); for (auto notifier : notifiers_) pollfds.push_back({ notifier.first, notifier.second.events(), 0 }); + pollfds.push_back({ eventfd_, POLLIN, 0 }); + /* Wait for events and process notifiers and timers. */ do { ret = poll(&pollfds); @@ -137,12 +150,20 @@ void EventDispatcherPoll::processEvents() ret = -errno; LOG(Event, Warning) << "poll() failed with " << strerror(-ret); } else if (ret > 0) { + processInterrupt(pollfds.back()); + pollfds.pop_back(); processNotifiers(pollfds); } processTimers(); } +void EventDispatcherPoll::interrupt() +{ + uint64_t value = 1; + write(eventfd_, &value, sizeof(value)); +} + short EventDispatcherPoll::EventNotifierSetPoll::events() const { short events = 0; @@ -186,6 +207,15 @@ int EventDispatcherPoll::poll(std::vector<struct pollfd> *pollfds) nextTimer ? &timeout : nullptr, nullptr); } +void EventDispatcherPoll::processInterrupt(const struct pollfd &pfd) +{ + if (!pfd.revents & POLLIN) + return; + + uint64_t value; + read(eventfd_, &value, sizeof(value)); +} + void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pollfds) { static const struct { diff --git a/src/libcamera/include/event_dispatcher_poll.h b/src/libcamera/include/event_dispatcher_poll.h index ac3efde082b4..1c0066c24dc8 100644 --- a/src/libcamera/include/event_dispatcher_poll.h +++ b/src/libcamera/include/event_dispatcher_poll.h @@ -31,6 +31,7 @@ public: void unregisterTimer(Timer *timer); void processEvents(); + void interrupt(); private: struct EventNotifierSetPoll { @@ -40,8 +41,10 @@ private: std::map<int, EventNotifierSetPoll> notifiers_; std::list<Timer *> timers_; + int eventfd_; int poll(std::vector<struct pollfd> *pollfds); + void processInterrupt(const struct pollfd &pfd); void processNotifiers(const std::vector<struct pollfd> &pollfds); void processTimers(); };
The new interrupt() function allows interrupting in-progress blocking processEvents() calls. This is useful to stop running event loops. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/event_dispatcher.h | 2 ++ src/libcamera/event_dispatcher.cpp | 10 ++++++ src/libcamera/event_dispatcher_poll.cpp | 32 ++++++++++++++++++- src/libcamera/include/event_dispatcher_poll.h | 3 ++ 4 files changed, 46 insertions(+), 1 deletion(-)