linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: William Mcvicker <willmcvicker@google.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Jessica Yu <jeyu@kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Michal Marek <michal.lkml@markovi.net>,
	linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
	kernel-team@android.com,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Saravana Kannan <saravanak@google.com>
Subject: Re: [PATCH v1 0/2] Add support to capture external module's SCM version
Date: Mon, 23 Nov 2020 14:13:38 -0800	[thread overview]
Message-ID: <20201123221338.GA2726675@google.com> (raw)
In-Reply-To: <20201123090257.GB6334@infradead.org>

On Mon, Nov 23, 2020 at 09:02:57AM +0000, Christoph Hellwig wrote:
> On Sat, Nov 21, 2020 at 01:16:49AM +0000, Will McVicker wrote:
> > These two patches add module support to capture an external module's SCM
> > version as a MODULE_INFO() attribute. This allows users to identity the SCM
> > version of a given kernel module by using the modinfo tool or on the device
> > via sysfs:
> 
> As this obviously is of no use for in-tree modules it falls under the we
> don't add code to support things that are not in tree rule and has no
> business in the kernel.

Hi Christoph,

Ah sorry, I didn't intend this to come across as only for external modules.
That just seemed like the easiest way to explain how the scmversion attribute
can be different from the vermagic. We mainly need this for in-tree kernel
modules since that's where most our drivers are. Let me re-phrase this with
that in mind. Basically, I like to look at this as an improved version of the
existing srcversion module attribute since it allows you to easily identify the
module version with a quick SCM version string check instead of doing a full
checksum on the module source.

For example, we have a setup to test kernel changes on the hikey and db845c
devices without updating the kernel modules. Without this scmversion module
attribute, you can't identify the original module version using `uname
-r`. And for kernel modules in the initramfs, you can't even use modinfo to get
the module vermagic.  With this patch, you are able to get the SCM version for
*all* kernel modules (on disk and in the initramfs) via the sysfs node:
/sys/module/<mod>/scmversion. This also works the other way around when
developers update their kernel modules to fix some bug (like a security
vulnerability) but don't need to update the full kernel.

Regarding the documentation, Greg, thanks for pointing out Documentation/ABI/!
I seached high and low for documentation on the other module sysfs attributes,
but couldn't find anything. I'll update the proper documentation in the v2
patchset.

Thanks,
Will


  reply	other threads:[~2020-11-23 22:13 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-21  1:16 [PATCH v1 0/2] Add support to capture external module's SCM version Will McVicker
2020-11-21  1:16 ` [PATCH v1 1/2] scripts/setlocalversion: allow running in a subdir Will McVicker
2020-11-21  1:16 ` [PATCH v1 2/2] modules: add scmversion field Will McVicker
2020-11-23  9:30   ` Greg KH
2020-11-23  9:32   ` Greg KH
2020-11-23  9:02 ` [PATCH v1 0/2] Add support to capture external module's SCM version Christoph Hellwig
2020-11-23 22:13   ` William Mcvicker [this message]
2020-11-24  9:31     ` Jessica Yu
2020-11-24 18:05       ` William Mcvicker
2020-11-24 18:12         ` Greg Kroah-Hartman
2020-11-24 18:31           ` William Mcvicker
2020-11-24 20:24             ` Greg Kroah-Hartman
2020-11-24 20:40               ` William Mcvicker
2020-11-24 20:45                 ` Saravana Kannan
2020-11-25  1:05                   ` [PATCH v2 0/2] Adds support to capture " Will McVicker
2020-11-25  1:05                     ` [PATCH v2 1/2] scripts/setlocalversion: allow running in a subdir Will McVicker
2020-11-25  1:05                     ` [PATCH v2 2/2] modules: add scmversion field Will McVicker
2020-12-07 15:31                       ` Jessica Yu
2020-12-08 20:05                         ` [PATCH v3 0/2] " Will McVicker
2020-12-08 20:05                           ` [PATCH v3 1/2] scripts/setlocalversion: allow running in a subdir Will McVicker
2020-12-11 15:33                             ` Jessica Yu
2020-12-16 22:08                               ` Will McVicker
2020-12-08 20:05                           ` [PATCH v3 2/2] modules: introduce the MODULE_SCMVERSION config Will McVicker
2020-12-04  0:36                     ` [PATCH v2 0/2] Adds support to capture module's SCM version William Mcvicker
2020-12-04  7:51                       ` Christoph Hellwig
2020-12-04 18:13                         ` Will McVicker
2020-12-04 18:18                           ` Christoph Hellwig
2020-12-04 18:20                             ` Will McVicker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201123221338.GA2726675@google.com \
    --to=willmcvicker@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@infradead.org \
    --cc=jeyu@kernel.org \
    --cc=kernel-team@android.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=michal.lkml@markovi.net \
    --cc=saravanak@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).