From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752584AbbEHG7b (ORCPT ); Fri, 8 May 2015 02:59:31 -0400 Received: from ozlabs.org ([103.22.144.67]:41905 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752190AbbEHG7V (ORCPT ); Fri, 8 May 2015 02:59:21 -0400 Message-ID: <1431068357.9673.9.camel@ellerman.id.au> Subject: Re: [PATCH 1/6] dump_stack: Support adding to the dump stack arch description From: Michael Ellerman To: Andrew Morton Cc: linuxppc-dev@ozlabs.org, linux-kernel@vger.kernel.org, Anton Blanchard , tj@kernel.org, Andrew Morton Date: Fri, 08 May 2015 16:59:17 +1000 In-Reply-To: <20150505141647.a1529e46f5ba42645264769c@linux-foundation.org> References: <1430824337-15339-1-git-send-email-mpe@ellerman.id.au> <20150505141647.a1529e46f5ba42645264769c@linux-foundation.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.10-0ubuntu1~14.10.1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2015-05-05 at 14:16 -0700, Andrew Morton wrote: > On Tue, 5 May 2015 21:12:12 +1000 Michael Ellerman 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