lkml.org 
[lkml]   [2020]   [Jun]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v8 03/13] perf evlist: implement control command handling functions
    From
    Date

    On 23.06.2020 17:56, Jiri Olsa wrote:
    > On Wed, Jun 17, 2020 at 11:36:46AM +0300, Alexey Budankov wrote:
    >
    > SNIP
    >
    >> +static int evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd,
    >> + char *cmd_data, size_t data_size)
    >> +{
    >> + int err;
    >> + char c;
    >> + size_t bytes_read = 0;
    >> +
    >> + memset(cmd_data, 0, data_size--);
    >> +
    >> + do {
    >> + err = read(evlist->ctl_fd.fd, &c, 1);
    >> + if (err > 0) {
    >> + if (c == '\n' || c == '\0')
    >> + break;
    >> + cmd_data[bytes_read++] = c;
    >> + if (bytes_read == data_size)
    >> + break;
    >> + } else {
    >> + if (err == -1)
    >> + pr_err("Failed to read from ctlfd %d: %m\n", evlist->ctl_fd.fd);
    >> + break;
    >> + }
    >> + } while (1);
    >> +
    >> + pr_debug("Message from ctl_fd: \"%s%s\"\n", cmd_data,
    >> + bytes_read == data_size ? "" : c == '\n' ? "\\n" : "\\0");
    >
    > do you want to display the message above only if (err > 0) ?

    I do, only for err >= 0. For err == -1 there is already the error message.

    >
    >> +
    >> + if (err > 0) {
    >> + if (!strncmp(cmd_data, EVLIST_CTL_CMD_ENABLE_TAG,
    >> + strlen(EVLIST_CTL_CMD_ENABLE_TAG))) {
    >> + *cmd = EVLIST_CTL_CMD_ENABLE;
    >> + } else if (!strncmp(cmd_data, EVLIST_CTL_CMD_DISABLE_TAG,
    >> + strlen(EVLIST_CTL_CMD_DISABLE_TAG))) {
    >> + *cmd = EVLIST_CTL_CMD_DISABLE;
    >> + }
    >> + }
    >> +
    >> + return err;
    >> +}
    >
    > SNIP
    >
    >> + int ctlfd_pos = evlist->ctl_fd.pos;
    >> + struct pollfd *entries = evlist->core.pollfd.entries;
    >> +
    >> + if (ctlfd_pos == -1 || !entries[ctlfd_pos].revents)
    >> + return 0;
    >> +
    >> + if (entries[ctlfd_pos].revents & POLLIN) {
    >> + err = evlist__ctlfd_recv(evlist, cmd, cmd_data,
    >> + EVLIST_CTL_CMD_MAX_LEN);
    >> + if (err > 0) {
    >> + switch (*cmd) {
    >> + case EVLIST_CTL_CMD_ENABLE:
    >> + evlist__enable(evlist);
    >> + break;
    >> + case EVLIST_CTL_CMD_DISABLE:
    >> + evlist__disable(evlist);
    >> + break;
    >> + case EVLIST_CTL_CMD_ACK:
    >> + case EVLIST_CTL_CMD_UNSUPPORTED:
    >> + default:
    >> + pr_debug("ctlfd: unsupported %d\n", *cmd);
    >> + break;
    >> + }
    >> + if (!(*cmd == EVLIST_CTL_CMD_ACK || *cmd == EVLIST_CTL_CMD_UNSUPPORTED))
    >> + evlist__ctlfd_ack(evlist);
    >> + }
    >> + }
    >> +
    >> + if (entries[ctlfd_pos].revents & (POLLHUP | POLLERR))
    >> + evlist__finalize_ctlfd(evlist);
    >
    > should this error be propagated via err?

    I don't think so. Currently it returns 0 only and there is no need to
    stop collection or skip processing in caller in this case.

    ~Alexey

    >
    > jirka
    >

    \
     
     \ /
      Last update: 2020-06-24 15:21    [W:4.168 / U:1.368 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site