On Thu, 27 Jul 2000 23:22:38 +0100 (BST),
Russell King <rmk@arm.linux.org.uk> wrote:
>Keith Owens writes:
>> Except for the places where cpu is a variable instead of smp_processor_id(),
>> those places might generate array access code.
>
>They shouldn't do - GCC should realise that code like:
>
> int this_cpu = 0;
>
> array_access[this_cpu].foo = 0;
> array_access[this_cpu].bar = 1;
>
>can be optimised. So long as "this_cpu" remains an auto variable, then
>it should be ok.
local_{bh,irq}_count are used from other macros, e.g. in_irq(),
irq_enter(), irq_exit(), hardirq_trylock(), hardirq_enter(),
hardirq_exit(), cpu_bh_disable() etc. I started to track down where
those macros are called from to make sure that all uses of cpu were
smp_processor_id() or auto variables but it was taking too long. So I
decided to go for the safe case and define separate macros. If the
work to track down all uses of cpu is done then the new definitions can
be consolidated, but for now - safety first.
>> I thought about using a
>> single set of macros and always defining irq_stat[NR_CPUS], trusting to
>> gcc to optimize the array access away for UP but decided to err on the
>> side of caution.
>
>We relied on it before, so why can't we rely on it again?
All the old definitions of irq_stat were [NR_CPUS]. But not all
architectures used irq_stat for local_{bh,irq}_count, some used a
single integer for UP and an array of ints for SMP. Some even used an
integer for UP and moved the counts into struct cpu_data[] for SMP. So
not all arch's relied on gcc optimization before.
>> As for readability, almost all the existing code
>> defines SMP and UP versions, at least this patch reduces to one
>> definition instead of one per arch with subtly different definitions.
>
>There's no argument there. Its only that it can be improved further
>with very little effort.
You are right but it will take time to confirm that the generated code
is the same for all uses of irq_stat. There are quite a few more arch
dependent definitions that can be consolidated like in_irq(), but I
left those for phase 2. The change was already fairly large and I
wanted to run it past the arch maintainers before spending any more
time on this.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Mon Jul 31 2000 - 21:00:25 EST