2009-01-28 13:37:05

by Rusty Russell

[permalink] [raw]
Subject: [PATCH 5/6] module: make modversion_info contain a pointer, not an array.


With allmodconfig (minus non-building modules) on 32-bit x86:
Total size of modules before: 60009790 bytes
Total size of modules after: 55927866 bytes

Saving 7% of module size for CONFIG_MODVERSIONS=y; and these sections
are kept resident as well.

Signed-off-by: Rusty Russell <[email protected]>
---
include/linux/module.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/module.h b/include/linux/module.h
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -37,10 +37,12 @@ struct kernel_symbol
const char *name;
};

+/* This is put in the __versions section of a module to indicate the version
+ * it expects for unknown symbols. */
struct modversion_info
{
unsigned long crc;
- char name[MODULE_NAME_LEN];
+ char *name;
};

struct module;


2009-01-28 14:53:30

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH 5/6] module: make modversion_info contain a pointer, not an array.

On Thu, 29 Jan 2009 00:05:52 +1030
Rusty Russell <[email protected]> wrote:

>
> With allmodconfig (minus non-building modules) on 32-bit x86:
> Total size of modules before: 60009790 bytes
> Total size of modules after: 55927866 bytes
>
> Saving 7% of module size for CONFIG_MODVERSIONS=y; and these sections
> are kept resident as well.
>

that reminds me.. can we just simplify MODVERSIONS to be a md5sum (or
sha1 whatver) of the .config file in the VERMAGIC ?
it's a lot more reliable in detecting incompatibilities, and a lot
less space consumed.

--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2009-01-28 22:29:56

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH 5/6] module: make modversion_info contain a pointer, not an array.

On Thursday 29 January 2009 01:22:31 Arjan van de Ven wrote:
> On Thu, 29 Jan 2009 00:05:52 +1030
> Rusty Russell <[email protected]> wrote:
>
> >
> > With allmodconfig (minus non-building modules) on 32-bit x86:
> > Total size of modules before: 60009790 bytes
> > Total size of modules after: 55927866 bytes
> >
> > Saving 7% of module size for CONFIG_MODVERSIONS=y; and these sections
> > are kept resident as well.
> >
>
> that reminds me.. can we just simplify MODVERSIONS to be a md5sum (or
> sha1 whatver) of the .config file in the VERMAGIC ?
> it's a lot more reliable in detecting incompatibilities, and a lot
> less space consumed.

Unfortunately people really seem to want the finer granularity that MODVERSIONS (sometimes) provides :( I've tried killing it off several times, and always failed.

We did discuss changing modversions not to descend more than one level deep into types (ie. MODVERSION(int fn(struct foo *)) would depend on the type of struct foo, but not the type of any struct pointers in struct foo) to reduce the problem where header changes cause type definitions to be exposed or hidden and thus change the modversion.

I asked someone to do some analysis on what effect this would have on signatures in real releases, but never heard back (it was in Austin last year, and there was beer...)

Cheers,
Rusty.



undefined types





>

2009-01-28 22:41:25

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH 5/6] module: make modversion_info contain a pointer, not an array.

On Thu, 29 Jan 2009 08:59:40 +1030
Rusty Russell <[email protected]> wrote:

> On Thursday 29 January 2009 01:22:31 Arjan van de Ven wrote:
> > On Thu, 29 Jan 2009 00:05:52 +1030
> > Rusty Russell <[email protected]> wrote:
> >
> > >
> > > With allmodconfig (minus non-building modules) on 32-bit x86:
> > > Total size of modules before: 60009790 bytes
> > > Total size of modules after: 55927866 bytes
> > >
> > > Saving 7% of module size for CONFIG_MODVERSIONS=y; and these
> > > sections are kept resident as well.
> > >
> >
> > that reminds me.. can we just simplify MODVERSIONS to be a md5sum
> > (or sha1 whatver) of the .config file in the VERMAGIC ?
> > it's a lot more reliable in detecting incompatibilities, and a lot
> > less space consumed.
>
> Unfortunately people really seem to want the finer granularity that
> MODVERSIONS (sometimes) provides :( I've tried killing it off
> several times, and always failed.

but we could just stick the result in VERMAGIC right?
rather than tacking it to every symbol.

how you calculate the global checksum is almost a separate debate.


--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2009-01-29 06:50:57

by Jon Masters

[permalink] [raw]
Subject: Re: [PATCH 5/6] module: make modversion_info contain a pointer, not an array.

On Thu, 2009-01-29 at 08:59 +1030, Rusty Russell wrote:

> We did discuss changing modversions not to descend more than
> one level deep into types (ie. MODVERSION(int fn(struct foo *))
> would depend on the type of struct foo, but not the type of any
> struct pointers in struct foo) to reduce the problem where header
> changes cause type definitions to be exposed or hidden and thus
> change the modversion.

Yeah. We looked at that for a certain Enterprise Linux distribution but
various people suggested it was more hassle than it was worth to get it
right. I did do some initial poking - I'll dig it up and post it here.

Jon.

2009-01-29 07:44:36

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH 5/6] module: make modversion_info contain a pointer, not an array.

On Thursday 29 January 2009 09:11:14 Arjan van de Ven wrote:
> On Thu, 29 Jan 2009 08:59:40 +1030
> Rusty Russell <[email protected]> wrote:
>
> > On Thursday 29 January 2009 01:22:31 Arjan van de Ven wrote:
> > > On Thu, 29 Jan 2009 00:05:52 +1030
> > > Rusty Russell <[email protected]> wrote:
> > >
> > > >
> > > > With allmodconfig (minus non-building modules) on 32-bit x86:
> > > > Total size of modules before: 60009790 bytes
> > > > Total size of modules after: 55927866 bytes
> > > >
> > > > Saving 7% of module size for CONFIG_MODVERSIONS=y; and these
> > > > sections are kept resident as well.
> > > >
> > >
> > > that reminds me.. can we just simplify MODVERSIONS to be a md5sum
> > > (or sha1 whatver) of the .config file in the VERMAGIC ?
> > > it's a lot more reliable in detecting incompatibilities, and a lot
> > > less space consumed.
> >
> > Unfortunately people really seem to want the finer granularity that
> > MODVERSIONS (sometimes) provides :( I've tried killing it off
> > several times, and always failed.
>
> but we could just stick the result in VERMAGIC right?
> rather than tacking it to every symbol.

Sure, but who would use it? If you add a driver I don't want my modules to fail.

Confused,
Rusty.