| Message ID | 20240925233854.90072-14-mmaurer@google.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Extended MODVERSIONS Support | expand |
| Context | Check | Description |
|---|---|---|
| mdraidci/vmtest-modules-next-PR | success | PR summary |
| mdraidci/vmtest-modules-next-VM_Test-0 | success | Logs for build-kernel |
On Wed, Sep 25, 2024 at 11:38:28PM +0000, Matthew Maurer wrote: > * modules.order has .o files when in a build dir, support this The commit log is not clear, is it that it's always had *.o files, and you're adding them now, why? Why is the .ko search now removed? > * .mod.c source layout has changed, When, why did this change not happen at that time? > update regexes to match Why did this not break anything before ? Is this fixing something, or is it prep work? > * Add a stage 3, to be more robust against additional .mod.c content Future .mod.c changes? Luis
On Fri, Oct 11, 2024 at 3:13 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Wed, Sep 25, 2024 at 11:38:28PM +0000, Matthew Maurer wrote: > > * modules.order has .o files when in a build dir, support this > > The commit log is not clear, is it that it's always had *.o files, and > you're adding them now, why? Why is the .ko search now removed? The script was broken when I found it, but it was a script that analyzed MODVERSIONS, so I tried to ensure it would still work with my changes. This necessitated rehabilitating it first. I did not touch `.modules.order` files, but they contained `.o` and so this script wouldn't run correctly. > > > * .mod.c source layout has changed, > > When, why did this change not happen at that time? It was changed for readability [1] back in 2019. I assume the change did not happen at that time because this script is rarely run. If we'd prefer to discard this patch and ignore the script instead (or remove it?), that's fine. [1]: https://lore.kernel.org/all/20190909113423.2289-2-yamada.masahiro@socionext.com/ > > > update regexes to match > > Why did this not break anything before ? Is this fixing something, or > is it prep work? > > > * Add a stage 3, to be more robust against additional .mod.c content > > Future .mod.c changes? The rest of this series adds additional `.mod.c` content to support the string names. This stage 3 is intended to prevent that from causing the script to choke. > > Luis
On Fri, Oct 11, 2024 at 03:22:54PM -0700, Matthew Maurer wrote: > On Fri, Oct 11, 2024 at 3:13 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > On Wed, Sep 25, 2024 at 11:38:28PM +0000, Matthew Maurer wrote: > > > * modules.order has .o files when in a build dir, support this > > > > The commit log is not clear, is it that it's always had *.o files, and > > you're adding them now, why? Why is the .ko search now removed? > > The script was broken when I found it, My point is that commit messages should describe all this. > but it was a script that > analyzed MODVERSIONS, so I tried to ensure it would still work with my > changes. OK yes please add this informaiton into the comit log. Also expand on it as to to *why* this was not a critical issue. > This necessitated rehabilitating it first. I did not touch > `.modules.order` files, but they contained `.o` and so this script > wouldn't run correctly. Just elaborate on the commit log. We cannot guess what is happening, and so bringing clarity into exactly what you are doing and *why* is helpful to determine the impact of not applying this. > > > * .mod.c source layout has changed, > > > > When, why did this change not happen at that time? > > It was changed for readability [1] back in 2019. I assume the change > did not happen at that time because this script is rarely run. This is crucial information to include in the commit log. When *would* you use it? So that use case was broken since 2019? What negative effects has this implicated? > If we'd > prefer to discard this patch and ignore the script instead (or remove > it?), that's fine. The feedback is for you to improve the commit logs, not to tell you to not do something. We cannot read your mind, and this smelled like a fix, but without a true evaluation and documentation of the impact of not having this in place. These things need to be thought out and written in the comit log. > > [1]: https://lore.kernel.org/all/20190909113423.2289-2-yamada.masahiro@socionext.com/ > > > > > > update regexes to match > > > > Why did this not break anything before ? Is this fixing something, or > > is it prep work? > > > > > * Add a stage 3, to be more robust against additional .mod.c content > > > > Future .mod.c changes? > > The rest of this series adds additional `.mod.c` content to support > the string names. This stage 3 is intended to prevent that from > causing the script to choke. The commit log should clarify all this. Luis
diff --git a/scripts/export_report.pl b/scripts/export_report.pl index feb3d5542a62..dcef915405f3 100755 --- a/scripts/export_report.pl +++ b/scripts/export_report.pl @@ -55,6 +55,7 @@ sub collectcfiles { open my $fh, '< modules.order' or die "cannot open modules.order: $!\n"; while (<$fh>) { s/\.ko$/.mod.c/; + s/\.o$/.mod.c/; push (@file, $_) } close($fh); @@ -120,10 +121,14 @@ foreach my $thismod (@allcfiles) { next; } if ($state == 1) { - $state = 2 if ($_ =~ /__attribute__\(\(section\("__versions"\)\)\)/); + $state = 2 if ($_ =~ /__used __section\("__versions"\)/); next; } if ($state == 2) { + if ( $_ =~ /};/ ) { + $state = 3; + next; + } if ( $_ !~ /0x[0-9a-f]+,/ ) { next; } @@ -133,7 +138,7 @@ foreach my $thismod (@allcfiles) { push(@{$MODULE{$thismod}} , $sym); } } - if ($state != 2) { + if ($state != 3) { warn "WARNING:$thismod is not built with CONFIG_MODVERSIONS enabled\n"; $modversion_warnings++; }
* modules.order has .o files when in a build dir, support this * .mod.c source layout has changed, update regexes to match * Add a stage 3, to be more robust against additional .mod.c content Signed-off-by: Matthew Maurer <mmaurer@google.com> --- scripts/export_report.pl | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)