From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrey Ryabinin Subject: Re: [patch V2 09/29] mm/kasan: Simplify stacktrace handling Date: Thu, 18 Apr 2019 13:39:45 +0300 Message-ID: <5b77992a-52b6-807e-f77d-9cf3e648c71f@virtuozzo.com> References: <20190418084119.056416939@linutronix.de> <20190418084253.903603121@linutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20190418084253.903603121@linutronix.de> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Thomas Gleixner , LKML Cc: Josh Poimboeuf , x86@kernel.org, Andy Lutomirski , Steven Rostedt , Alexander Potapenko , Dmitry Vyukov , kasan-dev@googlegroups.com, linux-mm@kvack.org, Alexey Dobriyan , Andrew Morton , Pekka Enberg , David Rientjes , Christoph Lameter , Catalin Marinas , Mike Rapoport , Akinobu Mita , iommu@lists.linux-foundation.org, Robin Murphy , Christoph Hellwig , Marek Szyprowski , Johannes Thumshirn List-Id: linux-arch.vger.kernel.org On 4/18/19 11:41 AM, Thomas Gleixner wrote: > Replace the indirection through struct stack_trace by using the storage > array based interfaces. > > Signed-off-by: Thomas Gleixner > Acked-by: Dmitry Vyukov > Cc: Andrey Ryabinin > Cc: Alexander Potapenko > Cc: kasan-dev@googlegroups.com > Cc: linux-mm@kvack.org Acked-by: Andrey Ryabinin > > static inline depot_stack_handle_t save_stack(gfp_t flags) > { > unsigned long entries[KASAN_STACK_DEPTH]; > - struct stack_trace trace = { > - .nr_entries = 0, > - .entries = entries, > - .max_entries = KASAN_STACK_DEPTH, > - .skip = 0 > - }; > + unsigned int nr_entries; > > - save_stack_trace(&trace); > - filter_irq_stacks(&trace); > - > - return depot_save_stack(&trace, flags); > + nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 0); > + nr_entries = filter_irq_stacks(entries, nr_entries); > + return stack_depot_save(entries, nr_entries, flags); Suggestion for further improvement: stack_trace_save() shouldn't unwind beyond irq entry point so we wouldn't need filter_irq_stacks(). Probably all call sites doesn't care about random stack above irq entry point, so it doesn't make sense to spend resources on unwinding non-irq stack from interrupt first an filtering out it later. It would improve performance of stack_trace_save() called from interrupt and fix page_owner which feed unfiltered stack to stack_depot_save(). Random non-irq part kills the benefit of using the stack_deopt_save(). From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay.sw.ru ([185.231.240.75]:53410 "EHLO relay.sw.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388369AbfDRKji (ORCPT ); Thu, 18 Apr 2019 06:39:38 -0400 Subject: Re: [patch V2 09/29] mm/kasan: Simplify stacktrace handling References: <20190418084119.056416939@linutronix.de> <20190418084253.903603121@linutronix.de> From: Andrey Ryabinin Message-ID: <5b77992a-52b6-807e-f77d-9cf3e648c71f@virtuozzo.com> Date: Thu, 18 Apr 2019 13:39:45 +0300 MIME-Version: 1.0 In-Reply-To: <20190418084253.903603121@linutronix.de> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-arch-owner@vger.kernel.org List-ID: To: Thomas Gleixner , LKML Cc: Josh Poimboeuf , x86@kernel.org, Andy Lutomirski , Steven Rostedt , Alexander Potapenko , Dmitry Vyukov , kasan-dev@googlegroups.com, linux-mm@kvack.org, Alexey Dobriyan , Andrew Morton , Pekka Enberg , David Rientjes , Christoph Lameter , Catalin Marinas , 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, dm-devel@redhat.com, Mike Snitzer , Alasdair Kergon , 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: <20190418103945.ysDhPBcRl-DNB2xkIjXYdlcV4lmZ5Hqm0kfrKsi6vLw@z> On 4/18/19 11:41 AM, Thomas Gleixner wrote: > Replace the indirection through struct stack_trace by using the storage > array based interfaces. > > Signed-off-by: Thomas Gleixner > Acked-by: Dmitry Vyukov > Cc: Andrey Ryabinin > Cc: Alexander Potapenko > Cc: kasan-dev@googlegroups.com > Cc: linux-mm@kvack.org Acked-by: Andrey Ryabinin > > static inline depot_stack_handle_t save_stack(gfp_t flags) > { > unsigned long entries[KASAN_STACK_DEPTH]; > - struct stack_trace trace = { > - .nr_entries = 0, > - .entries = entries, > - .max_entries = KASAN_STACK_DEPTH, > - .skip = 0 > - }; > + unsigned int nr_entries; > > - save_stack_trace(&trace); > - filter_irq_stacks(&trace); > - > - return depot_save_stack(&trace, flags); > + nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 0); > + nr_entries = filter_irq_stacks(entries, nr_entries); > + return stack_depot_save(entries, nr_entries, flags); Suggestion for further improvement: stack_trace_save() shouldn't unwind beyond irq entry point so we wouldn't need filter_irq_stacks(). Probably all call sites doesn't care about random stack above irq entry point, so it doesn't make sense to spend resources on unwinding non-irq stack from interrupt first an filtering out it later. It would improve performance of stack_trace_save() called from interrupt and fix page_owner which feed unfiltered stack to stack_depot_save(). Random non-irq part kills the benefit of using the stack_deopt_save().