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

[Metzger, Markus T]

>-----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/