On Thursday, May 6, 2010, Ingo Molnar <mingo@elte.hu> wrote:
quoted text
>
> * Cyrill Gorcunov <gorcunov@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?
--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/