Re: [PATCH] 2.5.44: lkcd (9/9): dump driver and build files

[Piet Delaney <piet@www.piet.net>]

On Tue, 2002-10-22 at 04:57, Keith Owens wrote:
> On Tue, 22 Oct 2002 14:47:45 -0400, 
> Christoph Hellwig <hch@sgi.com> wrote:
> >On Mon, Oct 21, 2002 at 03:06:59PM -0700, Piet Delaney wrote:
> >> > Using volatile is almost always a bug.  USe atomic variables
> >> > or bitops instead.
> >> 
> >> Yea, volatile is just being used to implement a simple atomic variable. 
> >
> >It just isn't guaranteed to be atomic.. Use atomic_t (for actual
> >values) or unsigned long + set_bit/test_bit/ænd friends for bitmasks.
> 
> atomic_t is problematic for debugging code which can be invoked by an
> error from any state.  On parisc, atomic_add is implemented using load
> and clear on a hash of the lock address, so it is possible to get
> collisions on locks when doing atomic ops from debugging code.
> Especially when the parisc code in 2.5.44 has exactly one hash table
> entry.  kdb has the same problem and tries to avoid atomic_t for the
> same reason, the current state is unreliable.
> 
> The dump_in_progress flag is set in one place and cleared in another.
> All the other uses of dump_in_progress are testing its state.  If
> atomic_t cannot be used safely, then it must be defined as volatile.

atomic_read(v) and atomic_set(v,i) seem to be safe for most platforms.

Looks like your right, for parisc even these functions are using a
spinlock. Is it really necessary for parisc to use spinlocks? Even
Solaris doesn't use spinlocks for atomic set and reads of atomic
variables.

-piet

-- 
piet@www.piet.net