RE: [PATCH] perf_counter: Fix a race on perf_counter_ctx

-----Original Message-----
From: Peter Zijlstra [mailto:peterz@xxxxxxxxxxxxx]
Sent: Tuesday, August 18, 2009 3:38 PM
To: Metzger, Markus T
Cc: Ingo Molnar; tglx@xxxxxxxxxxxxx; hpa@xxxxxxxxx; markus.t.metzger@xxxxxxxxx; linux-
kernel@xxxxxxxxxxxxxxx; Paul Mackerras
Subject: RE: [PATCH] perf_counter: Fix a race on perf_counter_ctx

On Tue, 2009-08-18 at 14:20 +0100, Metzger, Markus T wrote:
-----Original Message-----
From: Peter Zijlstra [mailto:peterz@xxxxxxxxxxxxx]
Sent: Tuesday, August 18, 2009 3:00 PM
To: Metzger, Markus T
Cc: Ingo Molnar; tglx@xxxxxxxxxxxxx; hpa@xxxxxxxxx; markus.t.metzger@xxxxxxxxxx; linux-
kernel@xxxxxxxxxxxxxxx; Paul Mackerras
Subject: RE: [PATCH] perf_counter: Fix a race on perf_counter_ctx

On Tue, 2009-08-18 at 14:59 +0200, Peter Zijlstra wrote:
On Tue, 2009-08-18 at 13:49 +0100, Metzger, Markus T wrote:
Hi Ingo, Peter,

Did you say that branch tracing is working for you?

On my system, the kernel hangs.

Could it be that it simply takes too long to copy the trace? When I set the number
of samples to 10, everything seems to work OK. When I increase that number to 1000,
the kernel is getting very slow and eventually hangs.

I get a message "hrtimer: interrupt too slow", and I get a soft lockup bug. The rest
of the message log seems pretty garbled.

How many NMI/s is this generating anyway?

One every 800 or so branches in the current configuration - which results in 800 plus
a few perf_counter_output() calls per interrupt.

Right, that's terribly expensive. It might be worth it to specialize
that.

int perf_bts_entry_size(struct perf_counter *counter)
{
u64 sample_type = counter->attr.sample_type;
int size = sizeof(perf_event_header);

if (sample_type & PERF_SAMPLE_IP)
size += sizeof(u64);

...

/* maybe disallow PERF_SAMPLE_CALLCHAIN/RAW and grouping
on BTS counters */

return size;
}

void perf_bts_output(struct perf_counter *counter, ...)
{
int size = perf_bts_entry_size(counter);
struct perf_output_handle handle;
u64 entry[size / sizeof(u64)];
int ret;

entry[0] = (struct perf_entry_header){
.type = PERF_EVENT_SAMPLE,
.misc = 0,
.size = size,
};

... /* set all entry things */

ret = perf_output_begin(&handle, counter,
size * nr_entries, 1, 1);
if (ret)
return;

for (i = 0; i < nr_entries; i++) {
/* over-write the two that differ per entry */
entry[ip_entry] = bts_data[i].ip;
entry[add_entry] = bts_data[i].the_other_one;

perf_output_copy(&handle, entry, size);
}

perf_output_end(&handle);
}

or something like that.. would that work?


Well, that would push out the limit a bit, but it would still be quite fragile.

Currently, I'm not sure that this (i.e. that the interrupt handling takes too long)
is the underlying problem of the hangs that I'm seeing.

If it truly is, then I would go with the two-buffer approach. This would make the
ISR itself predictably and reliably fast - at the expense of additional locked memory.

Do you think that this truly is the problem?
How would the kernel react if interrupts were disabled for too long? I would definitely
expect bad responsiveness, but can the kernel kill itself?

regards,
markus.

---------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen Germany
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 Ust.-IdNr.
VAT Registration No.: DE129385895
Citibank Frankfurt (BLZ 502 109 00) 600119052

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages

  • Time Drift Compensation on Linux Clusters
    ... While working on a Linux cluster with kernel version 2.4.27 we've ... for this problem which is based on the Pentium's TSC clock. ... Following is a detailed description of the problem and the fix, ... The number of interrupt per second is defined by ...
    (Linux-Kernel)
  • Re: pcm0 freezes computer with recent -STABLE
    ... Ian's fix for the USB problem also fixed the pcm0 hang ... CVSupping should fix it (I rebuilt my kernel with just this fix ... Save and restore the interrupt mask in fork1instead of using ... This should fix the boot-time USB hangs that have been reported. ...
    (freebsd-stable)
  • Re: Interrupt context...
    ... > gone through most of the posts on interrupt in usenet. ... > kernel stack and ISR is executed. ... More may be saved depending on the architecture. ... Here the kernel have assembler code to save all general ...
    (comp.os.linux.development.system)
  • Re: [PATCH 19-rc1] Fix typos in /Documentation : Misc
    ... +do not have a corresponding kernel virtual address space mapping) and ... This command sets the scale factor for the ABSOLUTE MOUSE POSITIONING mode. ... If you check the source code you will see that what I draw here as a frame ... interrupt-parent: contains the phandle of the interrupt ...
    (Linux-Kernel)
  • please pull from the trivial tree
    ... Fix spelling in E1000_DISABLE_PACKET_SPLIT Kconfig description ... +- Finding patch that caused a bug ... +Always try the latest kernel from kernel.org and build from source. ... Length of input string in bytes ...
    (Linux-Kernel)