2008-06-20 14:44:33

by Richard Kennedy

[permalink] [raw]
Subject: [PATCH] module: reorder struct module to save space on 64 bit builds

reorder struct module to save space on 64 bit builds.
saves 1 cacheline_size (128 on default x86_64 & 64 on AMD
Opteron/athlon) when CONFIG_MODULE_UNLOAD=y.

Signed-off-by: Richard Kennedy <[email protected]>
---

Patch against 2.6.26-rc6. tested & running successfully on AMD64 desktop
machine. This patch reduces the data segment of each module by 1
cacheline size.

I also compiled with this patch for 32 bit & there was no change in
size.

Richard




diff --git a/include/linux/module.h b/include/linux/module.h
index 3e03b1a..63f0eb6 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -249,27 +249,28 @@ struct module

/* Exported symbols */
const struct kernel_symbol *syms;
- unsigned int num_syms;
const unsigned long *crcs;
+ unsigned int num_syms;

/* GPL-only exported symbols. */
- const struct kernel_symbol *gpl_syms;
unsigned int num_gpl_syms;
+ const struct kernel_symbol *gpl_syms;
const unsigned long *gpl_crcs;

/* unused exported symbols. */
const struct kernel_symbol *unused_syms;
- unsigned int num_unused_syms;
const unsigned long *unused_crcs;
+ unsigned int num_unused_syms;
+
/* GPL-only, unused exported symbols. */
- const struct kernel_symbol *unused_gpl_syms;
unsigned int num_unused_gpl_syms;
+ const struct kernel_symbol *unused_gpl_syms;
const unsigned long *unused_gpl_crcs;

/* symbols that will be GPL-only in the near future. */
const struct kernel_symbol *gpl_future_syms;
- unsigned int num_gpl_future_syms;
const unsigned long *gpl_future_crcs;
+ unsigned int num_gpl_future_syms;

/* Exception table */
unsigned int num_exentries;
@@ -300,23 +301,9 @@ struct module

#ifdef CONFIG_GENERIC_BUG
/* Support for BUG */
+ unsigned num_bugs;
struct list_head bug_list;
struct bug_entry *bug_table;
- unsigned num_bugs;
-#endif
-
-#ifdef CONFIG_MODULE_UNLOAD
- /* Reference counts */
- struct module_ref ref[NR_CPUS];
-
- /* What modules depend on me? */
- struct list_head modules_which_use_me;
-
- /* Who is waiting for us to be unloaded */
- struct task_struct *waiter;
-
- /* Destruction function. */
- void (*exit)(void);
#endif

#ifdef CONFIG_KALLSYMS
@@ -342,6 +329,21 @@ struct module
struct marker *markers;
unsigned int num_markers;
#endif
+
+#ifdef CONFIG_MODULE_UNLOAD
+ /* What modules depend on me? */
+ struct list_head modules_which_use_me;
+
+ /* Who is waiting for us to be unloaded */
+ struct task_struct *waiter;
+
+ /* Destruction function. */
+ void (*exit)(void);
+
+ /* Reference counts */
+ struct module_ref ref[NR_CPUS];
+#endif
+
};
#ifndef MODULE_ARCH_INIT
#define MODULE_ARCH_INIT {}


2008-06-21 17:26:29

by Denys Vlasenko

[permalink] [raw]
Subject: Re: [PATCH] module: reorder struct module to save space on 64 bit builds

On Friday 20 June 2008 16:44, Richard Kennedy wrote:
> reorder struct module to save space on 64 bit builds.
> saves 1 cacheline_size (128 on default x86_64 & 64 on AMD
> Opteron/athlon) when CONFIG_MODULE_UNLOAD=y.
>
> Signed-off-by: Richard Kennedy <[email protected]>
> ---
>
> Patch against 2.6.26-rc6. tested & running successfully on AMD64 desktop
> machine. This patch reduces the data segment of each module by 1
> cacheline size.
>
> I also compiled with this patch for 32 bit & there was no change in
> size.

Sometime ago I did something similar. I also shrank the struct module
by ifdefing out fields which are not needed.

The patch appeared to fell through the cracks.

Here is it again with original submission text.

(Note: majoe reason for struct module's disproportionate size
is this member:

#ifdef CONFIG_MODULE_UNLOAD
/* Reference counts */
struct module_ref ref[NR_CPUS];

because every array member takes entire cacheline (by design):

struct module_ref
{
local_t count;
} ____cacheline_aligned;

I guess the solution is to not select CONFIG_MODULE_UNLOAD...


On Friday 14 September 2007 00:30, Denys Vlasenko wrote:
> Hi Andrew,
>
> module.c and module.h conatains code for finding
> exported symbols which are declared with EXPORT_UNUSED_SYMBOL,
> and this code is compiled in even if CONFIG_UNUSED_SYMBOLS is not set
> and thus there can be no EXPORT_UNUSED_SYMBOLs in modules anyway
> (because EXPORT_UNUSED_SYMBOL(x) are compiled out to nothing then).
>
> This patch adds required #ifdefs.
>
> This shrinks module.o and each *.ko file.
>
> Patch also regroups some struct module members so
> that on 64 bit CPUs we are not wasting 32 bits on padding here:
>
> const struct kernel_symbol *unused_syms;
> unsigned int num_unused_syms;
> const unsigned long *unused_crcs;
>
> It groups counters and pointers separately.
>
> Patch makes small edit to help text of CONFIG_MODULE_UNLOAD -
> it explicitly says that without that option, kernel
> will be also faster, not only "smaller and simpler".
> When I realized how much churn is going on under the hood
> in order to make module unloading possible, I felt that
> users are not informed well enough about it in the help text.
>
> And finally, structure members which hold length of module
> code (four such members there) and count of symbols
> are converted from longs to ints.
>
> We cannot possibly have a module where 32 bits won't
> be enough to hold such counts.
>
> For one, module loading checks module size for sanity
> before loading, so such insanely big module will fail
> that test first.
>
> In short, patch makes trivial changes which are "obviously correct"
> (famous last words).
>
> Patch is compile tested with various combinations of CONFIGs.
>
> Please put it into -mm.
>
> Signed-off-by: Denys Vlasenko <[email protected]>


Attachments:
(No filename) (2.84 kB)
linux-2.6.23-rc6.structmodule.patch (8.74 kB)
Download all attachments

2008-06-23 03:05:23

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] module: reorder struct module to save space on 64 bit builds

On Saturday 21 June 2008 00:44:11 Richard Kennedy wrote:
> reorder struct module to save space on 64 bit builds.
> saves 1 cacheline_size (128 on default x86_64 & 64 on AMD
> Opteron/athlon) when CONFIG_MODULE_UNLOAD=y.
>
> Signed-off-by: Richard Kennedy <[email protected]>

Thanks, applied!

Cheers,
Rusty.

2008-06-23 03:15:36

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] module: reorder struct module to save space on 64 bit builds

On Sunday 22 June 2008 03:26:15 Denys Vlasenko wrote:
> On Friday 20 June 2008 16:44, Richard Kennedy wrote:
> > reorder struct module to save space on 64 bit builds.
> > saves 1 cacheline_size (128 on default x86_64 & 64 on AMD
> > Opteron/athlon) when CONFIG_MODULE_UNLOAD=y.
> >
> > Signed-off-by: Richard Kennedy <[email protected]>
> > ---
> >
> > Patch against 2.6.26-rc6. tested & running successfully on AMD64 desktop
> > machine. This patch reduces the data segment of each module by 1
> > cacheline size.
> >
> > I also compiled with this patch for 32 bit & there was no change in
> > size.
>
> Sometime ago I did something similar. I also shrank the struct module
> by ifdefing out fields which are not needed.
>
> The patch appeared to fell through the cracks.
>
> Here is it again with original submission text.

Thanks, I've put this in my tree. There's some other module work going on, so
it might need a little rework.

Cheers,
Rusty.