On Wed, Mar 01, 2006 at 10:36:05PM +0100, Andi Kleen wrote:
> dgc@xxxxxxx (David Chinner) writes:
>
> > using a spinlock per cpu for superblock counter exclusion results in
> > a preempt counter overflow at 256p and above. Change the exclusion mechanism
> > to use atomic bit operations and busy wait loops to emulate the spin
> > lock exclusion mechanism but without the preempt count issues.
>
> That sounds like the totally wrong place to fix this.
> Wouldn't it be better to fix the spinlocks instead instead of hacking
> around this? After all any other code on that big box could run
> into the same issue.
The issue is one cpu taking all the spin locks for a mount point
to exclude per-cpu access while we do something global to the counter
(e.g a rebalance).
I initially used spinlocks because it was simple to implement, I
knew it would prevent preemption in the critical sections and I knew
of nothing that would prevent a single cpu from taking up to NR_CPU
+ 1 nested spinlocks. Turns out I made an incorrect assumption.
However, when we are excluding the counters, we are already running
atomic due to holding the in-core superblock spinlock and the
per-cpu code uses get_cpu()/put_cpu() so neither can get preempted
in the critical sections the spinlocks used to protect. So the only
considerations are having an atomic exclusion mechanism and a wait
method that does not sleep.
In hindsight, spinlocks were a bad implementation choice. If I knew
about the limits on the preempt counter in the first place, I would
not have used spinlocks. Instead, I would have used the mechanism that
I just checked in.
Cheers,
Dave.
--
Dave Chinner
R&D Software Enginner
SGI Australian Software Group
|