Messages in this thread Patch in this message | | | Subject | Re: [RFC][PATCH 2/2] perf: Add per event clockid support | From | Pawel Moll <> | Date | Fri, 20 Feb 2015 15:28:49 +0000 |
| |
On Fri, 2015-02-20 at 14:29 +0000, Peter Zijlstra wrote: > The below patch makes the distinction between these two cases by > adding perf_event_clock() which is used for the second case. It > further makes this configurable on a per-event basis, but adds a few > sanity checks such that we cannot combine events with different clocks > in confusing ways.
The idea works for me (obviously :-)
> And since we then have per-event configurability we might as well > retain the 'legacy' behaviour as a default.
Don't mind that at all.
> @@ -334,8 +335,7 @@ struct perf_event_attr { > */ > __u32 sample_stack_user; > > - /* Align to u64. */ > - __u32 __reserved_2; > + __u32 clockid;
I thought about it, but was sort-of-afraid to propose it :-)
Now, one thing I'm not 100% sure about it is it being unsigned, as clockid_t is signed for a reason (negative values have meaning - eg. dynamic clocks, which could be useful in some circumstances). Of course casting could be an answer, but is there any reason not to make it __s32?
> + default: > + /* XXX add: clock_id_valid() && clock_gettime_ns() ? */ > + err = -EINVAL; > + goto err_alloc; > + }
If you asked me, I'd say -EINVAL, no default.
Cheers!
Pawel
| |