public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Adrian Hunter <adrian.hunter@intel.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Andi Kleen <ak@linux.intel.com>,
	Alexey Budankov <alexey.budankov@linux.intel.com>,
	Namhyung Kim <namhyung@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/6] perf tools: Add FIFO file names as alternative options to --control
Date: Mon, 31 Aug 2020 15:19:27 +0200	[thread overview]
Message-ID: <20200831131927.GD406859@krava> (raw)
In-Reply-To: <20200829105015.10800-5-adrian.hunter@intel.com>

On Sat, Aug 29, 2020 at 01:50:13PM +0300, Adrian Hunter wrote:

SNIP

> +		*p = '\0';
> +
> +	/*
> +	 * O_RDWR avoids POLLHUPs which is necessary to allow the other
> +	 * end of a FIFO to be repeatedly opened and closed.
> +	 */
> +	fd = open(s, O_RDWR | O_NONBLOCK | O_CLOEXEC);
> +	if (fd < 0) {
> +		pr_err("Failed to open '%s'\n", s);
> +		ret = -errno;
> +		goto out_free;
> +	}
> +	*ctl_fd = fd;
> +	*ctl_fd_close = true;
> +
> +	if (p && *++p) {
> +		/* O_RDWR | O_NONBLOCK means the other end need not be open */
> +		fd = open(p, O_RDWR | O_NONBLOCK | O_CLOEXEC);
> +		if (fd < 0) {
> +			pr_err("Failed to open '%s'\n", p);
> +			ret = -errno;
> +			goto out_free;
> +		}
> +		*ctl_fd_ack = fd;
> +	}
> +
> +out_free:
> +	free(s);
> +	return ret;
> +}
> +
> +int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close)
>  {
>  	char *comma = NULL, *endptr = NULL;
>  
>  	if (strncmp(str, "fd:", 3))
> -		return -EINVAL;
> +		return evlist__parse_control_names(str, ctl_fd, ctl_fd_ack, ctl_fd_close);

do we want to mention somewhere that the fifo name is everything
except for 'fd:' ?

also how likely is that we will add another channel type that
will need another keyword (likd 'fd:')? I originaly thought
we'd use 'fifo:filename' for this ... would be great to somehow
avoid future confusions

thanks,
jirka


  reply	other threads:[~2020-08-31 13:19 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-29 10:50 [PATCH 0/6] perf tools: Enable AUX area tracing snapshots using a FIFO Adrian Hunter
2020-08-29 10:50 ` [PATCH 1/6] perf tools: Consolidate --control option parsing into one function Adrian Hunter
2020-08-29 10:50 ` [PATCH 2/6] perf tools: Handle read errors from ctl_fd Adrian Hunter
2020-08-29 10:50 ` [PATCH 3/6] perf tools: Use AsciiDoc formatting for --control option documentation Adrian Hunter
2020-08-29 10:50 ` [PATCH 4/6] perf tools: Add FIFO file names as alternative options to --control Adrian Hunter
2020-08-31 13:19   ` Jiri Olsa [this message]
2020-08-31 13:27     ` Adrian Hunter
2020-08-29 10:50 ` [PATCH 5/6] perf record: Add 'snapshot' control command Adrian Hunter
2020-08-31  6:23   ` Andi Kleen
2020-08-31  7:03     ` Adrian Hunter
2020-08-29 10:50 ` [PATCH 6/6] perf intel-pt: Document snapshot " Adrian Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200831131927.GD406859@krava \
    --to=jolsa@redhat.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=alexey.budankov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox