Re: [patch] reduce IPI noise due to /dev/cdrom open/close

[Nick Piggin <nickpiggin@yahoo.com.au>]

Andrew Morton wrote:
> On Tue, 04 Jul 2006 15:32:19 +1000
> Keith Owens <kaos@sgi.com> wrote:

>>Why raw_smp_processor_id?  That normally indicates code that wants a
>>lazy cpu number, but this code requires the exact cpu number, IMHO
>>using raw_smp_processor_id is confusing.  smp_processor_id can safely
>>be used here, bh_lru_lock has disabled irq or preempt.
> 
> 
> I expect raw_smp_processor_id() is used here as a a microoptimisation -
> avoid a might_sleep() which obviously will never trigger.

A microoptimisation because they've turned on DEBUG_PREEMPT and found
that smp_processor_id slows down? ;) Wouldn't it be better to just stick
to the normal rules (ie. what Keith said)?

It may be obvious in this case (though that doesn't help people who make
obvious mistakes, or mismerge patches) but this just seems like a nasty
precedent to set (or has it already been?).

> 
> But I think it'd be better to do just a single raw_smp_processor_id() for
> this entire function:
> 
>   static void bh_lru_install(struct buffer_head *bh)
>   {
> 	struct buffer_head *evictee = NULL;
> 	struct bh_lru *lru;
> +	int cpu;
> 
> 	check_irqs_on();
> 	bh_lru_lock();
> +	cpu = raw_smp_processor_id();
> -	lru = &__get_cpu_var(bh_lrus);
> +	lru = per_cpu(bh_lrus, cpu);
> 
> etcetera.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com