On Wed, Jun 24, 2020 at 05:55:02PM +0300, Alexey Budankov wrote: > > 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().
aaah it's set to zero in here: if (entries[ctlfd_pos].revents & (POLLHUP | POLLERR)) evlist__finalize_ctlfd(evlist); else entries[ctlfd_pos].revents = 0; yea, that's bad.. another reason to call it a hack jirka > > ~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 > >> >