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;
Date:   Wed, 3 Feb 2021 16:46:16 +0100
From:   Jessica Yu <jeyu@kernel.org>
To:     Will McVicker <willmcvicker@google.com>
Cc:     Masahiro Yamada <masahiroy@kernel.org>,
        Michal Marek <michal.lkml@markovi.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Christoph Hellwig <hch@infradead.org>,
        Saravana Kannan <saravanak@google.com>,
        linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
        kernel-team@android.com
Subject: Re: [PATCH v6] modules: introduce the MODULE_SCMVERSION config
Message-ID: <YBrFSLASG5yiqtZT@gunter>
References: <20210121213641.3477522-1-willmcvicker@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <20210121213641.3477522-1-willmcvicker@google.com>
Precedence: bulk

+++ Will McVicker [21/01/21 21:36 +0000]:
>Config MODULE_SCMVERSION introduces a new module attribute --
>`scmversion` -- which can be used to identify a given module's SCM
>version.  This is very useful for developers that update their kernel
>independently from their kernel modules or vice-versa since the SCM
>version provided by UTS_RELEASE (`uname -r`) will now differ from the
>module's vermagic attribute.
>
>For example, we have a CI setup that tests new kernel changes on the
>hikey960 and db845c devices without updating their kernel modules. When
>these tests fail, we need to be able to identify the exact device
>configuration the test was using. By including MODULE_SCMVERSION, we can
>identify the exact kernel and modules' SCM versions for debugging the
>failures.
>
>Additionally, by exposing the SCM version via the sysfs node
>/sys/module/MODULENAME/scmversion, one can also verify the SCM versions
>of the modules loaded from the initramfs. Currently, modinfo can only
>retrieve module attributes from the module's ko on disk and not from the
>actual module that is loaded in RAM.
>
>You can retrieve the SCM version in two ways,
>
>1) By using modinfo:
>    > modinfo -F scmversion MODULENAME
>2) By module sysfs node:
>    > cat /sys/module/MODULENAME/scmversion

Hi Will,

First off, thanks for being patient and being responsive throughout the patch
review process.

Personally, I am rather neutral towards this feature. We already provide
CONFIG_MODULE_SRCVERSION to provide a checksum of a module's source files and I
think the SCMVERSION is a nicer alternative. I can see how an optional
scmversion field might be helpful information for distro developers in testing
environments and in situations where it is possible for the kernel and modules
to be updated/packaged separately (for instance, the kernel selftest modules
under lib/ are in-tree modules that are provided as a separate kernel module
package in SLE).

Generally, out of principle, I do not want to merge a patch that's been NAK'd
repeatedly; even if I take the patch it'd likely be contested all the way up to
the merge window. So this boils down to whether Christoph (and maybe Greg) are
fine with this being a debug option that's not enabled by default.

Thanks,

Jessica