On 24.06.2020 17:00, Alexey Budankov wrote: > > On 23.06.2020 17:54, Jiri Olsa wrote: >> On Wed, Jun 17, 2020 at 11:43:58AM +0300, Alexey Budankov wrote: >>> >>> Implement handling of 'enable' and 'disable' control commands >>> coming from control file descriptor. >>> >>> Signed-off-by: Alexey Budankov <alexey.budan...@linux.intel.com> >>> --- >>> tools/perf/builtin-record.c | 16 ++++++++++++++++ >>> 1 file changed, 16 insertions(+) >>> >>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c >>> index d0b29a1070a0..0394e068dde8 100644 >>> --- a/tools/perf/builtin-record.c >>> +++ b/tools/perf/builtin-record.c >>> @@ -1527,6 +1527,7 @@ static int __cmd_record(struct record *rec, int argc, >>> const char **argv) >>> bool disabled = false, draining = false; >>> int fd; >>> float ratio = 0; >>> + enum evlist_ctl_cmd cmd = EVLIST_CTL_CMD_UNSUPPORTED; >>> >>> atexit(record__sig_exit); >>> signal(SIGCHLD, sig_handler); >>> @@ -1830,6 +1831,21 @@ static int __cmd_record(struct record *rec, int >>> argc, const char **argv) >>> alarm(rec->switch_output.time); >>> } >>> >>> + if (evlist__ctlfd_process(rec->evlist, &cmd) > 0) { >>> + switch (cmd) { >>> + case EVLIST_CTL_CMD_ENABLE: >>> + pr_info(EVLIST_ENABLED_MSG); >>> + break; >>> + case EVLIST_CTL_CMD_DISABLE: >>> + pr_info(EVLIST_DISABLED_MSG); >>> + break; >>> + case EVLIST_CTL_CMD_ACK: >>> + case EVLIST_CTL_CMD_UNSUPPORTED: >>> + default: >>> + break; >>> + } >>> + } >> >> so there's still the filter call like: >> >> if (evlist__filter_pollfd(rec->evlist, POLLERR | >> POLLHUP) == 0) >> draining = true; >> >> it will never be 0 if the control fds are stil alive no? > > Due to change in filter_pollfd() and preceding evlist__ctlfd_process() call > now control fd is not counted by filter_pollfd(). And evlist__ctlfd_process() still should be called second time right after evlist_poll() but prior filter_polfd().
~Alexey > > However event fds with .revents == 0 are not counted either and this breaks > the algorithm thus something more is still required to cover this gap. > > ~Alexey > >> >> jirka >>