Re: [PATCH 03/11] perf tool: Use PERF_EVENT_IOC_ID perf ioctl to read event id
On Mon, Oct 22, 2012 at 04:57:24PM +0900, Namhyung Kim wrote: > Hi, > > Just a minor nitpicking.. > > > On Sat, 20 Oct 2012 16:33:11 +0200, Jiri Olsa wrote: > > Changing the way we retrieve the event ID. Instead of parsing out > > the ID out of the read data, using the PERF_EVENT_IOC_ID ioctl. > > > > Keeping the old way in place to support kernels without > > PERF_EVENT_IOC_ID ioctl support. > [snip] > > + } else if (errno) > > + return -1; > > Is this check really needed? I think that returning non-zero from the > ioctl always sets the errno, no? How about this: > > ret = ioctl(fd, PERF_EVENT_IOC_ID, ); > if (!ret) > goto add; > > if (errno != ENOTTY) > return -1; > > ... > > I guess that it'll show you better diff stat. :) yep, looks better.. v2 candidate ;) thanks, jirka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/11] perf tool: Use PERF_EVENT_IOC_ID perf ioctl to read event id
Hi, Just a minor nitpicking.. On Sat, 20 Oct 2012 16:33:11 +0200, Jiri Olsa wrote: > Changing the way we retrieve the event ID. Instead of parsing out > the ID out of the read data, using the PERF_EVENT_IOC_ID ioctl. > > Keeping the old way in place to support kernels without > PERF_EVENT_IOC_ID ioctl support. [snip] > + } else if (errno) > + return -1; Is this check really needed? I think that returning non-zero from the ioctl always sets the errno, no? How about this: ret = ioctl(fd, PERF_EVENT_IOC_ID, ); if (!ret) goto add; if (errno != ENOTTY) return -1; ... I guess that it'll show you better diff stat. :) Thanks, Namhyung > > - perf_evlist__id_add(evlist, evsel, cpu, thread, read_data[id_idx]); > + add: > + perf_evlist__id_add(evlist, evsel, cpu, thread, id); > return 0; > } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/11] perf tool: Use PERF_EVENT_IOC_ID perf ioctl to read event id
Hi, Just a minor nitpicking.. On Sat, 20 Oct 2012 16:33:11 +0200, Jiri Olsa wrote: Changing the way we retrieve the event ID. Instead of parsing out the ID out of the read data, using the PERF_EVENT_IOC_ID ioctl. Keeping the old way in place to support kernels without PERF_EVENT_IOC_ID ioctl support. [snip] + } else if (errno) + return -1; Is this check really needed? I think that returning non-zero from the ioctl always sets the errno, no? How about this: ret = ioctl(fd, PERF_EVENT_IOC_ID, id); if (!ret) goto add; if (errno != ENOTTY) return -1; ... I guess that it'll show you better diff stat. :) Thanks, Namhyung - perf_evlist__id_add(evlist, evsel, cpu, thread, read_data[id_idx]); + add: + perf_evlist__id_add(evlist, evsel, cpu, thread, id); return 0; } -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/11] perf tool: Use PERF_EVENT_IOC_ID perf ioctl to read event id
On Mon, Oct 22, 2012 at 04:57:24PM +0900, Namhyung Kim wrote: Hi, Just a minor nitpicking.. On Sat, 20 Oct 2012 16:33:11 +0200, Jiri Olsa wrote: Changing the way we retrieve the event ID. Instead of parsing out the ID out of the read data, using the PERF_EVENT_IOC_ID ioctl. Keeping the old way in place to support kernels without PERF_EVENT_IOC_ID ioctl support. [snip] + } else if (errno) + return -1; Is this check really needed? I think that returning non-zero from the ioctl always sets the errno, no? How about this: ret = ioctl(fd, PERF_EVENT_IOC_ID, id); if (!ret) goto add; if (errno != ENOTTY) return -1; ... I guess that it'll show you better diff stat. :) yep, looks better.. v2 candidate ;) thanks, jirka -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
