Re: [RFC][PATCH -next] pstore: replace spin_lock withspin_trylock_irqsave in panic path

[Don Zickus]

On Tue, Sep 27, 2011 at 12:02:38PM -0700, Luck, Tony wrote:
> > Ok.  Do we care?  I assumed the panic data would be more
> > relevant/interesting than whatever pstore was doing before (like loading
> > previous log files).
> 
> Yes we care - saving panic data is most likely the single most important
> thing that pstore does.  I just have severe doubts that it will actually
> save anything useful if we just blindly continue if we can't get the lock.

Well, I was trying to imply that any pre-panic info is uninteresting.  It
is the panic/NMI stuff that should be top priority, worthy of busting the
spin lock.

> 
> What actually happens next will be dependent on the back-end. For
> the state machine in ERST, one possible outcome is a hang. For many
> people a hang is considered worse than a panic.

That should be up to the backend, no?  ERST has two modes, only one which
has a state machine.  The other is NVRAM which can probably handle
simultaneous writes.  And I believe the EFI back-end can handle that as
well.  That is why I was suggesting that the back-end return a failure.

> 
> > I assumed we are just overwriting the buffer with the current data, so
> > unless the other cpu is chugging along while this cpu is in panic, the new
> > data shouldn't get corrupted, no?
> 
> I really have no idea what *will* happen.  Lots of things are possible, only
> some of them are desirable.

My concern here is that if someone is just toying with pstore,
writing/reading data or even just poking at it to see what is going on
with the system, they may accidentally block real system errors or panics
from properly logging.  That doesn't seem right.

Cheers,
Don
--
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/