Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <161834460576.3764895.758141455860109099@swboyd.mtv.corp.google.com>
References: <20210410015300.3764485-1-swboyd@chromium.org> <20210410015300.3764485-6-swboyd@chromium.org> <YHQ1yqVkweZeN5+1@smile.fi.intel.com> <161825574550.3764895.4387100574176584209@swboyd.mtv.corp.google.com> <YHV4369VJAGpfW/c@smile.fi.intel.com> <YHW1xBvOeHrAHWkK@alley> <161834460576.3764895.758141455860109099@swboyd.mtv.corp.google.com>
Subject: Re: [PATCH v4 05/13] module: Add printk formats to add module build ID to stacktraces
From:   Stephen Boyd <swboyd@chromium.org>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org, Jiri Olsa <jolsa@kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Jessica Yu <jeyu@kernel.org>,
        Evan Green <evgreen@chromium.org>,
        Hsin-Yi Wang <hsinyi@chromium.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        linux-doc@vger.kernel.org, Matthew Wilcox <willy@infradead.org>
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Petr Mladek <pmladek@suse.com>
Date:   Tue, 13 Apr 2021 15:36:57 -0700
Message-ID: <161835341789.3764895.11018828620037960038@swboyd.mtv.corp.google.com>
User-Agent: alot/0.9.1
Precedence: bulk

Quoting Stephen Boyd (2021-04-13 13:10:05)
> Quoting Petr Mladek (2021-04-13 08:16:20)
> > On Tue 2021-04-13 13:56:31, Andy Shevchenko wrote:
> > > On Mon, Apr 12, 2021 at 12:29:05PM -0700, Stephen Boyd wrote:
> > > > Quoting Andy Shevchenko (2021-04-12 04:58:02)
> > > > >=20
> > > > > First of all, why not static_assert() defined near to the actual =
macro?
> > > >=20
> > > > Which macro? BUILD_ID_SIZE_MAX?
> > >=20
> > > Yes.
> > >=20
> > > > I tried static_assert() and it didn't
> > > > work for me but maybe I missed something.
> >=20
> > I guess that you wanted to use it inside macro definition:
> >=20
> > #define VMCOREINFO_BUILD_ID(value) \
> >         static_assert(ARRAY_SIZE(value) =3D=3D BUILD_ID_SIZE_MAX); \
> >         vmcoreinfo_append_str("BUILD-ID=3D%20phN\n", value)
> >=20
> > Instead, you should do it outside the macro:
> >=20
> > static_assert(ARRAY_SIZE(value) =3D=3D BUILD_ID_SIZE_MAX);
> > #define VMCOREINFO_BUILD_ID(value) \
> >         vmcoreinfo_append_str("BUILD-ID=3D%20phN\n", value)
>=20
> In this example "value" is not defined because it's an argument to the
> macro. How can this work?
>=20
> From what I can tell static_assert() is for the case that you want to
> assert something at the global scope level. BUILD_BUG_ON() can't be used
> at global scope. I see the usage is usually to assert struct members and
> alignment of those members. In turn, static_assert() can't be used at
> function level scope. Each has a use and in this case I want to assert
> at function level scope to be as close as possible to the place that
> would need to change.
>=20

Good news. I can do this to force a basic block and then GCC doesn't compla=
in.

---8<---
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index 2174dab16ba9..de62a722431e 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -38,9 +38,12 @@ phys_addr_t paddr_vmcoreinfo_note(void);

 #define VMCOREINFO_OSRELEASE(value) \
        vmcoreinfo_append_str("OSRELEASE=3D%s\n", value)
-#define VMCOREINFO_BUILD_ID(value) \
-       BUILD_BUG_ON(ARRAY_SIZE(value) !=3D BUILD_ID_SIZE_MAX); \
-       vmcoreinfo_append_str("BUILD-ID=3D%20phN\n", value)
+#define VMCOREINFO_BUILD_ID()                                          \
+       ({                                                              \
+               static_assert(sizeof(vmlinux_build_id) =3D=3D 20);         =
 \
+               vmcoreinfo_append_str("BUILD-ID=3D%20phN\n", vmlinux_build_=
id); \
+       })
+
 #define VMCOREINFO_PAGESIZE(value) \
        vmcoreinfo_append_str("PAGESIZE=3D%ld\n", value)