Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1650279 > unrolled thread

Re: [PATCH v2 13/13] perf tools: add feature header record to pipe-mode

Started byJiri Olsa <jolsa@redhat.com>
First post2017-05-25 10:20 +0200
Last post2017-06-06 20:20 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2 13/13] perf tools: add feature header record to  pipe-mode Jiri Olsa <jolsa@redhat.com> - 2017-05-25 10:20 +0200
    Re: [PATCH v2 13/13] perf tools: add feature header record to pipe-mode David Carrillo-Cisneros <davidcc@google.com> - 2017-06-06 03:40 +0200
      Re: [PATCH v2 13/13] perf tools: add feature header record to  pipe-mode Jiri Olsa <jolsa@redhat.com> - 2017-06-06 13:10 +0200
        Re: [PATCH v2 13/13] perf tools: add feature header record to pipe-mode David Carrillo-Cisneros <davidcc@google.com> - 2017-06-06 20:20 +0200

#1650279 — Re: [PATCH v2 13/13] perf tools: add feature header record to pipe-mode

FromJiri Olsa <jolsa@redhat.com>
Date2017-05-25 10:20 +0200
SubjectRe: [PATCH v2 13/13] perf tools: add feature header record to pipe-mode
Message-ID<tKW9X-5J4-9@gated-at.bofh.it>
On Tue, May 23, 2017 at 12:48:53AM -0700, David Carrillo-Cisneros wrote:

SNIP

> +int perf_event__synthesize_features(struct perf_tool *tool,
> +				    struct perf_session *session,
> +				    struct perf_evlist *evlist,
> +				    perf_event__handler_t process)
> +{
> +	struct perf_header *header = &session->header;
> +	struct feat_fd fdd;
> +	struct feature_event *fe;
> +	size_t sz, sz_hdr;
> +	int feat, ret;
> +
> +	sz_hdr = sizeof(fe->header);
> +	sz = sizeof(union perf_event);
> +	/* get a nice alignment */
> +	sz = PERF_ALIGN(sz, getpagesize());
> +
> +	memset(&fdd, 0, sizeof(fdd));
> +
> +	fdd.buf = malloc(sz);
> +	if (!fdd.buf)
> +		return -ENOMEM;
> +
> +	fdd.size = sz - sz_hdr;
> +
> +	for_each_set_bit(feat, header->adds_features, HEADER_FEAT_BITS) {
> +		if (!feat_ops[feat].has_record) {
> +			pr_debug("No record header feature for header :%d\n", feat);
> +			continue;
> +		}
> +
> +		fdd.offset = sizeof(*fe);
> +
> +		ret = feat_ops[feat].write(&fdd, evlist);
> +		if (ret || fdd.offset <= (ssize_t)sizeof(*fe)) {
> +			pr_debug("Error writing feature\n");
> +			continue;
> +		}
> +
> +		/* fdd.buf may have changed due to realloc in do_write() */

right, so how's ensured the data never cross the maximum event size (0xffff) ?

I think do_write should have some check on that

jirka

[toc] | [next] | [standalone]


#1658306 — Re: [PATCH v2 13/13] perf tools: add feature header record to pipe-mode

FromDavid Carrillo-Cisneros <davidcc@google.com>
Date2017-06-06 03:40 +0200
SubjectRe: [PATCH v2 13/13] perf tools: add feature header record to pipe-mode
Message-ID<tPbDr-24f-3@gated-at.bofh.it>
In reply to#1650279
On Thu, May 25, 2017 at 1:10 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Tue, May 23, 2017 at 12:48:53AM -0700, David Carrillo-Cisneros wrote:
>
> SNIP
>
>> +int perf_event__synthesize_features(struct perf_tool *tool,
>> +                                 struct perf_session *session,
>> +                                 struct perf_evlist *evlist,
>> +                                 perf_event__handler_t process)
>> +{
>> +     struct perf_header *header = &session->header;
>> +     struct feat_fd fdd;
>> +     struct feature_event *fe;
>> +     size_t sz, sz_hdr;
>> +     int feat, ret;
>> +
>> +     sz_hdr = sizeof(fe->header);
>> +     sz = sizeof(union perf_event);
>> +     /* get a nice alignment */
>> +     sz = PERF_ALIGN(sz, getpagesize());
>> +
>> +     memset(&fdd, 0, sizeof(fdd));
>> +
>> +     fdd.buf = malloc(sz);
>> +     if (!fdd.buf)
>> +             return -ENOMEM;
>> +
>> +     fdd.size = sz - sz_hdr;
>> +
>> +     for_each_set_bit(feat, header->adds_features, HEADER_FEAT_BITS) {
>> +             if (!feat_ops[feat].has_record) {
>> +                     pr_debug("No record header feature for header :%d\n", feat);
>> +                     continue;
>> +             }
>> +
>> +             fdd.offset = sizeof(*fe);
>> +
>> +             ret = feat_ops[feat].write(&fdd, evlist);
>> +             if (ret || fdd.offset <= (ssize_t)sizeof(*fe)) {
>> +                     pr_debug("Error writing feature\n");
>> +                     continue;
>> +             }
>> +
>> +             /* fdd.buf may have changed due to realloc in do_write() */
>
> right, so how's ensured the data never cross the maximum event size (0xffff) ?
>
> I think do_write should have some check on that

do_write reallocates ff->buff when it's not large enough.

>
> jirka

[toc] | [prev] | [next] | [standalone]


#1658636

FromJiri Olsa <jolsa@redhat.com>
Date2017-06-06 13:10 +0200
Message-ID<tPkx3-7Uz-17@gated-at.bofh.it>
In reply to#1658306
On Mon, Jun 05, 2017 at 06:32:50PM -0700, David Carrillo-Cisneros wrote:
> On Thu, May 25, 2017 at 1:10 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> > On Tue, May 23, 2017 at 12:48:53AM -0700, David Carrillo-Cisneros wrote:
> >
> > SNIP
> >
> >> +int perf_event__synthesize_features(struct perf_tool *tool,
> >> +                                 struct perf_session *session,
> >> +                                 struct perf_evlist *evlist,
> >> +                                 perf_event__handler_t process)
> >> +{
> >> +     struct perf_header *header = &session->header;
> >> +     struct feat_fd fdd;
> >> +     struct feature_event *fe;
> >> +     size_t sz, sz_hdr;
> >> +     int feat, ret;
> >> +
> >> +     sz_hdr = sizeof(fe->header);
> >> +     sz = sizeof(union perf_event);
> >> +     /* get a nice alignment */
> >> +     sz = PERF_ALIGN(sz, getpagesize());
> >> +
> >> +     memset(&fdd, 0, sizeof(fdd));
> >> +
> >> +     fdd.buf = malloc(sz);
> >> +     if (!fdd.buf)
> >> +             return -ENOMEM;
> >> +
> >> +     fdd.size = sz - sz_hdr;
> >> +
> >> +     for_each_set_bit(feat, header->adds_features, HEADER_FEAT_BITS) {
> >> +             if (!feat_ops[feat].has_record) {
> >> +                     pr_debug("No record header feature for header :%d\n", feat);
> >> +                     continue;
> >> +             }
> >> +
> >> +             fdd.offset = sizeof(*fe);
> >> +
> >> +             ret = feat_ops[feat].write(&fdd, evlist);
> >> +             if (ret || fdd.offset <= (ssize_t)sizeof(*fe)) {
> >> +                     pr_debug("Error writing feature\n");
> >> +                     continue;
> >> +             }
> >> +
> >> +             /* fdd.buf may have changed due to realloc in do_write() */
> >
> > right, so how's ensured the data never cross the maximum event size (0xffff) ?
> >
> > I think do_write should have some check on that
> 
> do_write reallocates ff->buff when it's not large enough.

and what if it's bigger than 0xffff?

jirka

[toc] | [prev] | [next] | [standalone]


#1659030 — Re: [PATCH v2 13/13] perf tools: add feature header record to pipe-mode

FromDavid Carrillo-Cisneros <davidcc@google.com>
Date2017-06-06 20:20 +0200
SubjectRe: [PATCH v2 13/13] perf tools: add feature header record to pipe-mode
Message-ID<tPrfc-3TW-1@gated-at.bofh.it>
In reply to#1658636
On Tue, Jun 6, 2017 at 4:04 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Mon, Jun 05, 2017 at 06:32:50PM -0700, David Carrillo-Cisneros wrote:
>> On Thu, May 25, 2017 at 1:10 AM, Jiri Olsa <jolsa@redhat.com> wrote:
>> > On Tue, May 23, 2017 at 12:48:53AM -0700, David Carrillo-Cisneros wrote:
>> >
>> > SNIP
>> >
>> >> +int perf_event__synthesize_features(struct perf_tool *tool,
>> >> +                                 struct perf_session *session,
>> >> +                                 struct perf_evlist *evlist,
>> >> +                                 perf_event__handler_t process)
>> >> +{
>> >> +     struct perf_header *header = &session->header;
>> >> +     struct feat_fd fdd;
>> >> +     struct feature_event *fe;
>> >> +     size_t sz, sz_hdr;
>> >> +     int feat, ret;
>> >> +
>> >> +     sz_hdr = sizeof(fe->header);
>> >> +     sz = sizeof(union perf_event);
>> >> +     /* get a nice alignment */
>> >> +     sz = PERF_ALIGN(sz, getpagesize());
>> >> +
>> >> +     memset(&fdd, 0, sizeof(fdd));
>> >> +
>> >> +     fdd.buf = malloc(sz);
>> >> +     if (!fdd.buf)
>> >> +             return -ENOMEM;
>> >> +
>> >> +     fdd.size = sz - sz_hdr;
>> >> +
>> >> +     for_each_set_bit(feat, header->adds_features, HEADER_FEAT_BITS) {
>> >> +             if (!feat_ops[feat].has_record) {
>> >> +                     pr_debug("No record header feature for header :%d\n", feat);
>> >> +                     continue;
>> >> +             }
>> >> +
>> >> +             fdd.offset = sizeof(*fe);
>> >> +
>> >> +             ret = feat_ops[feat].write(&fdd, evlist);
>> >> +             if (ret || fdd.offset <= (ssize_t)sizeof(*fe)) {
>> >> +                     pr_debug("Error writing feature\n");
>> >> +                     continue;
>> >> +             }
>> >> +
>> >> +             /* fdd.buf may have changed due to realloc in do_write() */
>> >
>> > right, so how's ensured the data never cross the maximum event size (0xffff) ?
>> >
>> > I think do_write should have some check on that
>>
>> do_write reallocates ff->buff when it's not large enough.
>
> and what if it's bigger than 0xffff?
>
Oh yeah, I'll add that check.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web