Messages in this thread | | | Date | Tue, 18 Oct 2011 08:49:29 -0400 | From | Don Zickus <> | Subject | Re: [RFC][PATCH -next] make pstore/kmsg_dump run after stopping other cpus in panic path |
| |
On Mon, Oct 17, 2011 at 04:47:15PM -0700, Andrew Morton wrote: > On Fri, 14 Oct 2011 16:53:05 -0400 > > @@ -131,11 +133,7 @@ static void pstore_dump(struct kmsg_dumper *dumper, > > total += l1_cpy + l2_cpy; > > part++; > > } > > - if (in_nmi()) { > > - if (is_locked) > > - spin_unlock(&psinfo->buf_lock); > > - } else > > - spin_unlock_irqrestore(&psinfo->buf_lock, flags); > > + spin_unlock_irqrestore(&psinfo->buf_lock, flags); > > } > > afacit this assumes that (reason == KMSG_DUMP_PANIC) if in_nmi(). Is > that always the case, and will it always be the case in the future?
I see your point. For now yes. The common case for which I think pstore was designed for was the APEI/GHES case. Normally when GHES hits an NMI it stays at the APEI/GHES layer and either uses an irq_workqueue for recoverable errors or panics on non-recoverable errors.
So currently the only time it reaches the pstore layer is in the panic case. Unfortunately, I can't vouch for all the backends that can hook into pstore.
Perhaps a 'BUG_ON(in_nmi() && reason != KMSG_DUMP_PANIC)'?
> > I felt that the spin_trylock() approach was less horrid than this. I > assume that the new approach will cause lockdep to go berzerk?
Heh. Good point. That is probably a good test case. Though finding a working GHES implementation in the firmware isn't easy these days, making it hard to test. :-/
Cheers, Don
| |