Re: [PATCH v3 1/7] dump_stack: Support adding to the dump stack arch description

On Wed 2019-02-20 14:44:33, Andrea Parri wrote:

quoted

quoted

quoted

quoted

quoted

+		 * Order the stores above in vsnprintf() vs the store of the
+		 * space below which joins the two strings. Note this doesn't
+		 * make the code truly race free because there is no barrier on
+		 * the read side. ie. Another CPU might load the uninitialised
+		 * tail of the buffer first and then the space below (rather
+		 * than the NULL that was there previously), and so print the
+		 * uninitialised tail. But the whole string lives in BSS so in
+		 * practice it should just see NULLs.

It is not my intention to support concurrent updates of the string. The
idea is you setup the string early in boot.

Understood, thanks for the clarification.

quoted

The concern with a concurrent reader is simply that the string is dumped
in the panic path, and you never really know when you're going to panic.
Even if you only write to the string before doing SMP bringup you might
still have another CPU go rogue and panic before then.

But I probably should have just not added the barrier, it's over
paranoid and will almost certainly never matter in practice.

Oh, well, I can only echo you: if you don't care about the stores being
_observed_ out of order, you could simply remove the barrier; if you do
care, then you need "more paranoid" on the readers side.  ;-)

Hmm, the barrier might be fine and actually useful. The
purpose is to make sure that the later '\0' is written before
the existing one is replaced by ' '.

The reader does not need the barrier as long as it reads the string
sequentially. I would expect that it is always the case. But who
knows with all the speculation-related CPU bugs around.

In each case, any race could never crash the kernel.
The dump_stack_arch_desc_str is zeroed out of box and
the very last '\0' is never rewritten.

Best Regards,
Petr