Re: [RFC][PATCH v2 -next 2/2] Adding lock operations tokmsg_dump()/pstore_dump()

From: Don Zickus
Date: Fri Oct 28 2011 - 16:01:26 EST


On Fri, Oct 28, 2011 at 12:02:15PM -0700, Luck, Tony wrote:
> > It ain't pretty but it moves things towards a more reliable message dump.
> > The odds of us needing to bust the spinlocks are really small. Most of
> > the time no one reads the pstore filesystem.
>
> Does it really change the odds much? As you say, the common case is that
> pstore front-end doesn't have the lock held - so that case is unchanged.
> We can get the lock anyway, we don't need to bust it.

Agreed.

>
> Looking at the uncommon case where the lock is held - that means that
> pstore was in the middle of some back-end operation. Busting the lock
> means that the back-end will be surprised by being called again when the
> first operation had not yet completed. In the case of a state machine
> driven back end like ERST, I don't think this has a high probability of
> working out well.

Remember ERST has two modes, state machine and NVRAM. The state machine
will have issues, but the NVRAM part (which isn't implemented yet) might
not. Not sure about EFI. But shouldn't the backend determine that, not pstore?

>
> So you might be moving the needle from 99.999% chance of saving to pstore
> with 0.001% chance of hanging on the spin lock. to 99.9991% chance of
> saving, and 0.0009% chance of something highly weird happening in the
> back-end driver because you busted the lock and called it anyway.

Sure. But at the same time, APEI is one of those 'value add' by OEMs. If
you are paying $20K for this feature, wouldn't you expect this feature to
work 100% of the time? At least with kdump/netdump, you can say, hey it
was free so you get what you pay for.

I guess it would help if we had more machines with working firmware to
test this.

>
> > I would love to figure out a prettier solution for this locking mess, but
> > I can't think of anything. We have customers who want to utilize this
> > technology, so I am trying to make sure it is stable and robust for now.
> > A little selfish I suppose. But we are open to ideas?
>
> If a prettier solution is needed - it will have to involve the back-end.
> Perhaps a whole separate write/panic path (with separate buffer). Then
> a sufficiently smart back end could do the right thing. I have little
> confidence that ERST could be made smart in this way, because almost all
> of the heavy lifting is done by the BIOS - so Linux has no way to influence
> the flow of execution.

Sadly I agree.

Perhaps I have been hanging around mjg too much, but I little confidence
in anything ACPI related being smart.

I don't have that much motivation to push this patch very hard. I just
saw some a theoretical issue and thought I could help Seiji solve it. I
am more interested in getting the first patch of this series in than this one.

If you find this patch adds more complexity for very little gain, so be
it. We tried. :-)

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/