From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 39695C433ED for ; Thu, 15 Apr 2021 08:53:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EDF2961074 for ; Thu, 15 Apr 2021 08:53:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231500AbhDOIx2 (ORCPT ); Thu, 15 Apr 2021 04:53:28 -0400 Received: from mx2.suse.de ([195.135.220.15]:59584 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231251AbhDOIxZ (ORCPT ); Thu, 15 Apr 2021 04:53:25 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1618476781; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=WXKLpoBi6rAmdX0EzavGiuPYe0Kt3AGJje04jsBjUDs=; b=XRcRbNYR3DMpEIasLAZr8INrdzaOGiT411ZwU0XialkYsLwTFVrZyACUJGkFu8xAMgQdcI yqKFVZFVIrj7ZH1oG6OPvIJZeHcxlXUB66q9TUp63x/rNGU8HlWAmmZbO5idv7YdQgOM6E rUHKq1dNiwk/vyqrMuXvVcxtEeSR4mU= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 404FEB1F7; Thu, 15 Apr 2021 08:53:01 +0000 (UTC) Date: Thu, 15 Apr 2021 10:53:00 +0200 From: Petr Mladek To: Stephen Boyd Cc: Andrew Morton , linux-kernel@vger.kernel.org, Jiri Olsa , Alexei Starovoitov , Jessica Yu , Evan Green , Hsin-Yi Wang , Steven Rostedt , Sergey Senozhatsky , Andy Shevchenko , Rasmus Villemoes , linux-doc@vger.kernel.org, Matthew Wilcox Subject: Re: [PATCH v4 05/13] module: Add printk formats to add module build ID to stacktraces Message-ID: References: <20210410015300.3764485-1-swboyd@chromium.org> <20210410015300.3764485-6-swboyd@chromium.org> <161835466995.3764895.13268854960596303989@swboyd.mtv.corp.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <161835466995.3764895.13268854960596303989@swboyd.mtv.corp.google.com> Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org On Tue 2021-04-13 15:57:49, Stephen Boyd wrote: > Quoting Petr Mladek (2021-04-13 08:01:14) > > On Fri 2021-04-09 18:52:52, Stephen Boyd wrote: > > > Let's make kernel stacktraces easier to identify by including the build > > > ID[1] of a module if the stacktrace is printing a symbol from a module. > > > This makes it simpler for developers to locate a kernel module's full > > > debuginfo for a particular stacktrace. Combined with > > > scripts/decode_stracktrace.sh, a developer can download the matching > > > debuginfo from a debuginfod[2] server and find the exact file and line > > > number for the functions plus offsets in a stacktrace that match the > > > module. This is especially useful for pstore crash debugging where the > > > kernel crashes are recorded in something like console-ramoops and the > > > recovery kernel/modules are different or the debuginfo doesn't exist on > > > the device due to space concerns (the debuginfo can be too large for > > > space limited devices). > > > > > > diff --git a/include/linux/module.h b/include/linux/module.h > > > index 59f094fa6f74..4bf869f6c944 100644 > > > --- a/include/linux/module.h > > > +++ b/include/linux/module.h > > > @@ -11,6 +11,7 @@ > > > > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -367,6 +368,9 @@ struct module { > > > /* Unique handle for this module */ > > > char name[MODULE_NAME_LEN]; > > > > > > + /* Module build ID */ > > > + unsigned char build_id[BUILD_ID_SIZE_MAX]; > > > > Do we want to initialize/store the ID even when > > CONFIG_STACKTRACE_BUILD_ID is disabled and nobody would > > use it? > > > > Most struct module members are added only when the related feature > > is enabled. > > > > I am not sure how it would complicate the code. It is possible > > that it is not worth it. Well, I could imagine that the API > > will always pass the buildid parameter and > > module_address_lookup() might do something like > > > > #ifndef CONFIG_STACKTRACE_BUILD_ID > > static char empty_build_id[BUILD_ID_SIZE_MAX]; > > #endif > > > > if (modbuildid) { > > if (IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)) > > *modbuildid = mod->build_id; > > else > > *modbuildid = empty_build_id; > > > > IMHO, this is primary a call for Jessica as the module code maintainer. > > > > Otherwise, I am fine with this patch. And it works as expected. > > > > Does declaring mod->build_id as zero length work well enough? It might be fine because it would actually never get displayed. But yeah, it is kind of hack. The idea was to avoid too many #ifdefs in the code. I think that it is Jessica's call what she would prefer. Best Regards, Petr