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

From: Nick Piggin
Date: Tue Jul 04 2006 - 13:33:42 EST


Andrew Morton wrote:
On Tue, 04 Jul 2006 15:32:19 +1000
Keith Owens <kaos@xxxxxxx> 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 -
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/