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

[Michael Ellerman <mpe@ellerman.id.au>]

On Tue, 2015-05-05 at 14:16 -0700, Andrew Morton wrote:
> On Tue,  5 May 2015 21:12:12 +1000 Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> > Arch code can set a "dump stack arch description string" which is
> > displayed with oops output to describe the hardware platform.
> > +
> > +	len = strnlen(dump_stack_arch_desc_str, sizeof(dump_stack_arch_desc_str));
> > +	pos = len;
> > +
> > +	if (len)
> > +		pos++;
> > +
> > +	if (pos >= sizeof(dump_stack_arch_desc_str))
> > +		return; /* Ran out of space */
> > +
> > +	p = &dump_stack_arch_desc_str[pos];
> > +
> > +	va_start(args, fmt);
> > +	vsnprintf(p, sizeof(dump_stack_arch_desc_str) - pos, fmt, args);
> > +	va_end(args);
> 
> This code is almost race-free.  A (documented) smp_wmb() in here would
> make that 100%?
> 
> > +	if (len)
> > +		dump_stack_arch_desc_str[len] = ' ';
> > +}

On second thoughts I don't think it would.

It would order the stores in vsnprintf() vs the store of the space. The idea
being you never see a partially printed string. But for that to actually work
you need a barrier on the read side, and where do you put it?

The cpu printing the buffer could speculate the load of the tail of the buffer,
seeing something half printed from vsnprintf(), and then load the head of the
buffer and see the space, unless you order those loads.

So I don't think we can prevent a crashing cpu seeing a semi-printed buffer
without a lock, and we don't want to add a lock.

The other issue would be that a reader could miss the trailing NULL from the
vsnprintf() but see the space, meaning it would wander off the end of the
buffer. But the buffer's in BSS to start with, and we're careful not to print
off the end of it, so it should always be NULL terminated.

cheers