Prev: amended: first round of vhost-net enhancements for net-next
Next: usb gadget webcam: depends on VIDEO_DEV
From: Cyrill Gorcunov on 6 May 2010 03:50
On Thursday, May 6, 2010, Ingo Molnar <mingo(a)elte.hu> wrote:
>
> * Cyrill Gorcunov <gorcunov(a)gmail.com> wrote:
>
>> On Wed, May 05, 2010 at 06:57:34PM +0200, Frederic Weisbecker wrote:
>> ...
>> > > @@ -741,7 +743,7 @@ static int p4_pmu_schedule_events(struct
>> > > �{
>> > > � unsigned long used_mask[BITS_TO_LONGS(X86_PMC_IDX_MAX)];
>> > > � unsigned long escr_mask[BITS_TO_LONGS(ARCH_P4_TOTAL_ESCR)];
>> > > - int cpu = raw_smp_processor_id();
>> > > + int cpu = get_cpu();
>> > > � struct hw_perf_event *hwc;
>> > > � struct p4_event_bind *bind;
>> > > � unsigned int i, thread, num;
>> > > @@ -777,6 +779,7 @@ reserve:
>> > > � }
>> > >
>> > > �done:
>> > > + put_cpu();
>> > > � return num ? -ENOSPC : 0;
>> > > �}
>> >
>> > That's no big deal. But I think the schedule_events() is called on
>> > pmu::enable() time, when preemption is already disabled.
>> >
>>
>> We'll be on a safe side using get/put_cpu here (ie in case
>> if something get changed one day).
>
> hm, when 'something gets changed one day' we'll see a warning when using
> unsafe primitives.
>
> So if preemption is always off here we really should not add extra runtime
> overhead via get_cpu()/put_cpu().
>
> So wouldnt it be better (and faster) to disable preemption in
> hw_perf_event_init(), which seems to be the bit missing?
>
> � � � �Ingo
>

the thing are that p4 is only snippet here which is sensible to
preemtion, and hw_perf_event_init is executing with preemtion off (but
i could miss the details here, dont have code under my hands at
moment, so PeterZ help is needed ;) but more important reason why i've
saved get/put here is that otherwise i would not have rights to put
tested-by tag, since it would not be the patch Steven has tested. We
could make a patch on top of this one, or we could drop this one, make
new with explicit preemt off in caller and use smp_processor_id in p4
schedule routine. What is preferred?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: amended: first round of vhost-net enhancements for net-next
Next: usb gadget webcam: depends on VIDEO_DEV