From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ingo Molnar Subject: Re: [patch V3 00/29] stacktrace: Consolidate stack trace usage Date: Thu, 25 Apr 2019 12:09:33 +0200 Message-ID: <20190425100933.GB8387@gmail.com> References: <20190425094453.875139013@linutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20190425094453.875139013@linutronix.de> Sender: linux-kernel-owner@vger.kernel.org To: Thomas Gleixner Cc: LKML , Josh Poimboeuf , x86@kernel.org, Andy Lutomirski , Steven Rostedt , Alexander Potapenko , Alexey Dobriyan , Andrew Morton , Christoph Lameter , Pekka Enberg , linux-mm@kvack.org, David Rientjes , Catalin Marinas , Dmitry Vyukov , Andrey Ryabinin , kasan-dev@googlegroups.com, Mike Rapoport , Akinobu Mita , Christoph Hellwig , iommu@lists.linux-foundation.org, Robin Murphy List-Id: linux-arch.vger.kernel.org * Thomas Gleixner wrote: > - if (unlikely(!ret)) > + if (unlikely(!ret)) { > + if (!trace->nr_entries) { > + /* > + * If save_trace fails here, the printing might > + * trigger a WARN but because of the !nr_entries it > + * should not do bad things. > + */ > + save_trace(trace); > + } > return print_circular_bug(&this, target_entry, next, prev); > + } > else if (unlikely(ret < 0)) > return print_bfs_bug(ret); Just a minor style nit: the 'else' should probably on the same line as the '}' it belongs to, to make it really obvious that the 'if' has an 'else' branch? At that point the condition should probably also use balanced curly braces. Interdiff looks good otherwise. Thanks, Ingo From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f53.google.com ([209.85.128.53]:39793 "EHLO mail-wm1-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726247AbfDYKJj (ORCPT ); Thu, 25 Apr 2019 06:09:39 -0400 Date: Thu, 25 Apr 2019 12:09:33 +0200 From: Ingo Molnar Subject: Re: [patch V3 00/29] stacktrace: Consolidate stack trace usage Message-ID: <20190425100933.GB8387@gmail.com> References: <20190425094453.875139013@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190425094453.875139013@linutronix.de> Sender: linux-arch-owner@vger.kernel.org List-ID: To: Thomas Gleixner Cc: LKML , Josh Poimboeuf , x86@kernel.org, Andy Lutomirski , Steven Rostedt , Alexander Potapenko , Alexey Dobriyan , Andrew Morton , Christoph Lameter , Pekka Enberg , linux-mm@kvack.org, David Rientjes , Catalin Marinas , Dmitry Vyukov , Andrey Ryabinin , kasan-dev@googlegroups.com, Mike Rapoport , Akinobu Mita , Christoph Hellwig , iommu@lists.linux-foundation.org, Robin Murphy , Marek Szyprowski , Johannes Thumshirn , David Sterba , Chris Mason , Josef Bacik , linux-btrfs@vger.kernel.org, dm-devel@redhat.com, Mike Snitzer , Alasdair Kergon , Daniel Vetter , intel-gfx@lists.freedesktop.org, Joonas Lahtinen , Maarten Lankhorst , dri-devel@lists.freedesktop.org, David Airlie , Jani Nikula , Rodrigo Vivi , Tom Zanussi , Miroslav Benes , linux-arch@vger.kernel.org Message-ID: <20190425100933.UjqFqKzkSIWg4xdRFye2nWZWLPH4oDnA2NybsU1yseg@z> * Thomas Gleixner wrote: > - if (unlikely(!ret)) > + if (unlikely(!ret)) { > + if (!trace->nr_entries) { > + /* > + * If save_trace fails here, the printing might > + * trigger a WARN but because of the !nr_entries it > + * should not do bad things. > + */ > + save_trace(trace); > + } > return print_circular_bug(&this, target_entry, next, prev); > + } > else if (unlikely(ret < 0)) > return print_bfs_bug(ret); Just a minor style nit: the 'else' should probably on the same line as the '}' it belongs to, to make it really obvious that the 'if' has an 'else' branch? At that point the condition should probably also use balanced curly braces. Interdiff looks good otherwise. Thanks, Ingo