From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Rostedt Subject: Re: [patch V2 14/29] dm bufio: Simplify stack trace retrieval Date: Thu, 18 Apr 2019 09:33:56 -0400 Message-ID: <20190418093356.5d4b7732@gandalf.local.home> References: <20190418084119.056416939@linutronix.de> <20190418084254.361284697@linutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Alexander Potapenko Cc: Thomas Gleixner , LKML , Josh Poimboeuf , x86@kernel.org, Andy Lutomirski , dm-devel@redhat.com, Mike Snitzer , Alasdair Kergon , Alexey Dobriyan , Andrew Morton , Pekka Enberg , Linux Memory Management List , David Rientjes , Christoph Lameter , Catalin Marinas , Dmitry Vyukov , Andrey Ryabinin , kasan-dev , Mike Rapoport , Akinobu Mita , iommu@lists.linu List-Id: linux-arch.vger.kernel.org On Thu, 18 Apr 2019 14:11:44 +0200 Alexander Potapenko wrote: > On Thu, Apr 18, 2019 at 1:54 PM Thomas Gleixner wrote: > > > > On Thu, 18 Apr 2019, Alexander Potapenko wrote: > > > On Thu, Apr 18, 2019 at 11:06 AM Thomas Gleixner wrote: > > > > - save_stack_trace(&b->stack_trace); > > > > + b->stack_len = stack_trace_save(b->stack_entries, MAX_STACK, 2); > > > As noted in one of similar patches before, can we have an inline > > > comment to indicate what does this "2" stand for? > > > > Come on. We have gazillion of functions which take numerical constant > > arguments. Should we add comments to all of them? > Ok, sorry. I might not be familiar enough with the kernel style guide. It is a legitimate complaint but not for this series. I only complain about hard coded constants when they are added. That "2" was not added by this series. This patch set is a clean up of the stack tracing code, not a clean up of removing hard coded constants, or commenting them. The hard coded "2" was there without a comment before this patch series and Thomas is correct to leave it as is for these changes. This patch series should not modify what was already there which is out of scope for the purpose of these changes. A separate clean up patch to the maintainer of the subsystem (dm bufio in this case) is fine. But it's not Thomas's responsibility. -- Steve From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.kernel.org ([198.145.29.99]:37160 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727807AbfDRNeB (ORCPT ); Thu, 18 Apr 2019 09:34:01 -0400 Date: Thu, 18 Apr 2019 09:33:56 -0400 From: Steven Rostedt Subject: Re: [patch V2 14/29] dm bufio: Simplify stack trace retrieval Message-ID: <20190418093356.5d4b7732@gandalf.local.home> In-Reply-To: References: <20190418084119.056416939@linutronix.de> <20190418084254.361284697@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-arch-owner@vger.kernel.org List-ID: To: Alexander Potapenko Cc: Thomas Gleixner , LKML , Josh Poimboeuf , x86@kernel.org, Andy Lutomirski , dm-devel@redhat.com, Mike Snitzer , Alasdair Kergon , Alexey Dobriyan , Andrew Morton , Pekka Enberg , Linux Memory Management List , David Rientjes , Christoph Lameter , Catalin Marinas , Dmitry Vyukov , Andrey Ryabinin , kasan-dev , Mike Rapoport , Akinobu Mita , iommu@lists.linux-foundation.org, Robin Murphy , Christoph Hellwig , Marek Szyprowski , Johannes Thumshirn , David Sterba , Chris Mason , Josef Bacik , linux-btrfs@vger.kernel.org, intel-gfx@lists.freedesktop.org, Joonas Lahtinen , Maarten Lankhorst , dri-devel@lists.freedesktop.org, David Airlie , Jani Nikula , Daniel Vetter , Rodrigo Vivi , linux-arch@vger.kernel.org Message-ID: <20190418133356.Kg-Bey2ZEtrfSpFS1p-sEQJ98xtgAzTZRUDXeiuDpcg@z> On Thu, 18 Apr 2019 14:11:44 +0200 Alexander Potapenko wrote: > On Thu, Apr 18, 2019 at 1:54 PM Thomas Gleixner wrote: > > > > On Thu, 18 Apr 2019, Alexander Potapenko wrote: > > > On Thu, Apr 18, 2019 at 11:06 AM Thomas Gleixner wrote: > > > > - save_stack_trace(&b->stack_trace); > > > > + b->stack_len = stack_trace_save(b->stack_entries, MAX_STACK, 2); > > > As noted in one of similar patches before, can we have an inline > > > comment to indicate what does this "2" stand for? > > > > Come on. We have gazillion of functions which take numerical constant > > arguments. Should we add comments to all of them? > Ok, sorry. I might not be familiar enough with the kernel style guide. It is a legitimate complaint but not for this series. I only complain about hard coded constants when they are added. That "2" was not added by this series. This patch set is a clean up of the stack tracing code, not a clean up of removing hard coded constants, or commenting them. The hard coded "2" was there without a comment before this patch series and Thomas is correct to leave it as is for these changes. This patch series should not modify what was already there which is out of scope for the purpose of these changes. A separate clean up patch to the maintainer of the subsystem (dm bufio in this case) is fine. But it's not Thomas's responsibility. -- Steve