Messages in this thread | | | Subject | Re: [PATCH 2/3] perf tools: Allow to enable/disable events via control file | From | Alexei Budankov <> | Date | Thu, 10 Dec 2020 21:20:42 +0300 |
| |
On 10.12.2020 21:06, Jiri Olsa wrote: > On Thu, Dec 10, 2020 at 05:24:30PM +0100, Jiri Olsa wrote: >> On Mon, Dec 07, 2020 at 08:02:20PM +0300, Alexei Budankov wrote: >>> Hi, >>> >>> On 06.12.2020 20:05, Jiri Olsa wrote: >>>> Adding new control events to enable/disable specific event. >>>> The interface string for control file are: >>>> >>>> 'enable-<EVENT NAME>' >>>> 'disable-<EVENT NAME>' >>> >>> <SNIP> >>> >>>> >>>> when received the command, perf will scan the current evlist >>>> for <EVENT NAME> and if found it's enabled/disabled. >>> >>> <SNIP> >>> >>>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c >>>> index 70aff26612a9..05723227bebf 100644 >>>> --- a/tools/perf/util/evlist.c >>>> +++ b/tools/perf/util/evlist.c >>>> @@ -1915,7 +1915,13 @@ static int evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd, >>>> bytes_read == data_size ? "" : c == '\n' ? "\\n" : "\\0"); >>>> >>>> if (bytes_read > 0) { >>>> - if (!strncmp(cmd_data, EVLIST_CTL_CMD_ENABLE_TAG, >>>> + if (!strncmp(cmd_data, EVLIST_CTL_CMD_ENABLE_EVSEL_TAG, >>>> + (sizeof(EVLIST_CTL_CMD_ENABLE_EVSEL_TAG)-1))) { >>>> + *cmd = EVLIST_CTL_CMD_ENABLE_EVSEL; >>>> + } else if (!strncmp(cmd_data, EVLIST_CTL_CMD_DISABLE_EVSEL_TAG, >>>> + (sizeof(EVLIST_CTL_CMD_DISABLE_EVSEL_TAG)-1))) { >>>> + *cmd = EVLIST_CTL_CMD_DISABLE_EVSEL; >>>> + } else if (!strncmp(cmd_data, EVLIST_CTL_CMD_ENABLE_TAG, >>>> (sizeof(EVLIST_CTL_CMD_ENABLE_TAG)-1))) { >>>> *cmd = EVLIST_CTL_CMD_ENABLE; >>>> } else if (!strncmp(cmd_data, EVLIST_CTL_CMD_DISABLE_TAG, >>>> @@ -1952,6 +1958,8 @@ int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd) >>>> char cmd_data[EVLIST_CTL_CMD_MAX_LEN]; >>>> int ctlfd_pos = evlist->ctl_fd.pos; >>>> struct pollfd *entries = evlist->core.pollfd.entries; >>>> + struct evsel *evsel; >>>> + char *evsel_name; >>>> >>>> if (!evlist__ctlfd_initialized(evlist) || !entries[ctlfd_pos].revents) >>>> return 0; >>>> @@ -1967,6 +1975,26 @@ int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd) >>>> case EVLIST_CTL_CMD_DISABLE: >>>> evlist__disable(evlist); >>>> break; >>>> + case EVLIST_CTL_CMD_ENABLE_EVSEL: >>>> + evsel_name = cmd_data + sizeof(EVLIST_CTL_CMD_ENABLE_EVSEL_TAG) - 1; >>> >>> It makes sense to check that evsel_name still points >>> into cmd_data buffer after assigning to event name. >> >> right, will add that > > actualy it's already checked in evlist__ctlfd_recv, evsel_name at > worst will be empty string so evlist__find_evsel_by_str will fail > > I'll add '' around %s in the error output string: > > failed: can't find '%s' event > > so it's obvious when it's empty
Looks good to me. Thanks!
Alexei
> > jirka > > . >
| |