lkml.org 
[lkml]   [2020]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v12 05/15] perf evlist: implement control command handling functions
Em Wed, Jul 22, 2020 at 09:27:34AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Jul 17, 2020 at 10:01:33AM +0300, Alexey Budankov escreveu:
> >
> > Implement functions of initialization, finalization and processing
> > of control command messages coming from control file descriptors.
> > Allocate control file descriptor as descriptor at struct pollfd
> > object of evsel_list for atomic poll() operation.
> >
> > Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> > Acked-by: Jiri Olsa <jolsa@redhat.com>
> > Acked-by: Namhyung Kim <namhyung@kernel.org>
>
> I'm applying locally for testing, but can you answer what is the reason
> for that newline in the CMD_ACK_TAG? I tried looking at previous
> discussion but couldn't quickly find any reasoning for that
>
> - Arnaldo
>
> > +++ b/tools/perf/util/evlist.h
> > @@ -359,4 +359,22 @@ void perf_evlist__force_leader(struct evlist *evlist);
> > struct evsel *perf_evlist__reset_weak_group(struct evlist *evlist,
> > struct evsel *evsel,
> > bool close);
> > +#define EVLIST_CTL_CMD_ENABLE_TAG "enable"
> > +#define EVLIST_CTL_CMD_DISABLE_TAG "disable"
> > +#define EVLIST_CTL_CMD_ACK_TAG "ack\n"
>
> What for? That '\n'?

Oh, I see, this is not a _command_, it is a _response_, and the sender
expects a newline, right?

So I think this needs to be marked clearly as:

// Commands accepted by 'perf record'

#define EVLIST_CTL_CMD_ENABLE_TAG "enable"
#define EVLIST_CTL_CMD_DISABLE_TAG "disable"


// Responses sent by 'perf record'

#define EVLIST_CTL_RSP_ACK "ack"

And then make the response add the newline, as conceivably we could have
something like "nack", "invalid", "enoperm", etc, no?

Anyway, this can be done on top of this series, so I'm continuing
processing it locally.

- Arnaldo

\
 
 \ /
  Last update: 2020-07-22 14:35    [W:0.053 / U:0.700 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site