lkml.org 
[lkml]   [2020]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RESEND PATCH V3] perf/x86: Consider pinned events for group validation
On Thu, Jan 16, 2020 at 11:00:25AM -0800, kan.liang@linux.intel.com wrote:
> @@ -2054,9 +2057,38 @@ static int validate_group(struct perf_event *event)
> if (n < 0)
> goto out;
>
> + /*
> + * Disable interrupts and preemption to prevent the events in this
> + * CPU's cpuc going away and getting freed.
> + */
> + local_irq_save(flags);
> +
> + /*
> + * The new group must can be scheduled together with current pinned
> + * events. Otherwise, it will never get a chance to be scheduled later.
> + *
> + * It won't catch all possible cases that cannot schedule, such as
> + * events pinned on CPU1, but the validation for a new CPU1 event
> + * running on other CPU. However, it's good enough to handle common
> + * cases like the global NMI watchdog.
> + */
> + for (i = 0; i < cpuc->n_events; i++) {
> + pinned_event = cpuc->event_list[i];
> + if (WARN_ON_ONCE(!pinned_event))
> + continue;
> + if (!pinned_event->attr.pinned)
> + continue;
> + fake_cpuc->n_events = n;
> + n = collect_events(fake_cpuc, pinned_event, false);
> + if (n < 0)
> + goto irq;
> + }
> +

So I still completely hate this, because it makes the counter scheduling
more eratic.

It changes a situation where we only have false-positives (we allow
scheduling a group that might not ever get to run) into a situation
where we can have both false-positives and false-negatives.

Imagine the pinned event is for a currently running task; and that task
only runs sporadically. Then you can sometimes not create the group, but
mostly it'll work.

Yes, this is all very annoying, but I really don't see how this makes
anything any better.

\
 
 \ /
  Last update: 2020-01-17 10:14    [W:0.067 / U:1.616 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site