Re: [PATCH] perf_event_open.2: 4.9 PERF_EVENT_IOC_SET_DRV_CONFIGS support

On 12 August 2016 at 14:31, Vince Weaver [off-list ref] wrote:

On Fri, 12 Aug 2016, Mathieu Poirier wrote:

quoted

I am adding an interface to pass PMU specific configuration to the
driver.  Since PMU drivers exist for different architecture and
drivers I am making the mechanism as generic as possible.

It's a shame we are ending up with two "string configuration" ioctls.
PERF_EVENT_IOC_SET_FILTER and this new one.  Though I guess
PERF_EVENT_IOC_SET_FILTER is not really set up to be used generically.

quoted

"@cfg" and "@cfg=option".  The lexer will strip off the '@' and pass
"cfg" and "cfg=option" to the kernel.

What gets sent down to the kernel is driver specific - it is up to the
PMU drivers to parse and validate what's given to them.

But the core kernel is parsing "=" and "," in this case, so it's not
entirely up to the PMU driver, right?

Upon Peter's request the core will does some initial parsing but the
PMU drivers are responsible to parse the "cfg" and "option" parts in
order to validate their content.

Is there going to be a list of allowed keywords somewhere, under /sys
or similar?

SysFS is very constraining - I think it would be much better in the
PMU's documentation under Documentation/.  I have a patch ready for
that.

quoted

quoted

.TP
.BR PERF_EVENT_IOC_SET_DRV_CONFIGS " (since Linux 4.9)"
Pass custom configuration paramaters to a PMU driver.

The argument is a pointer to a NUL-termiated string of up to
PAGE_SIZE in length.
The string contains a list of comma-separated configuration options
that will be parsed by the kernel.
The kernel handles both singleton values as well as name/value pairs
that are indicated with the '=' character.
The size of the strings is limited internally to PERF_DRV_CONFIG_MAX
(which is not visible to userspace).

quoted

Yes this is a very good description but in sharp contrast with what is
currently done for the ioctl() descriptors in this page.  I shied away
from writing that much based on how slim the current descriptions were.

Well that's because this interface is a lot more complex than some of the
other ioctls which just take a simple integer (or no argument at all).

The ftrace ioctl description could definitely use some expansion.

I agree.

quoted

quoted

some additional questions: can this ioctl be run at any time or should
only be run while the event is quiet?  Does the change in options take
place immediately?

quoted

That is up to PMU drivers to decide - for CoreSight it is set only
once when trace sessions are created.  Any changes from thereon will
be ignored.

Then shouldn't this be set at perf_event_open() and not by an ioctl()?

Using an ioctl() seemed to be the best approach since all the
infrastructure is already there.  Also different drivers may need to
see configuration options updated - it is simply not the case for
CoreSight.

By the way, my next patchset will make PERF_DRV_CONFIG_MAX visible to
user space.

Thanks,
Mathieu

Vince

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html