public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Denys Vlasenko <vda.linux@googlemail.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] module.c: add missing ifdefs for CONFIG_UNUSED_SYMBOLS
Date: Thu, 13 Sep 2007 16:00:47 -0700	[thread overview]
Message-ID: <20070913160047.159e5e3d.akpm@linux-foundation.org> (raw)
In-Reply-To: <200709132330.33283.vda.linux@googlemail.com>

On Thu, 13 Sep 2007 23:30:33 +0100
Denys Vlasenko <vda.linux@googlemail.com> wrote:

> 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).

The intent seems reasonable.  Would have preferred separate patches for the
separate things though..

This:

akpm:/usr/src/25> grep '^+#' patches/modulec-add-missing-ifdefs-for-config_unused_symbols.patch                                                                 +#ifdef CONFIG_UNUSED_SYMBOLS
+#endif
+#ifdef CONFIG_UNUSED_SYMBOLS
+#endif
+#ifdef CONFIG_UNUSED_SYMBOLS
+#endif
+#ifdef CONFIG_UNUSED_SYMBOLS
+#endif
+#ifdef CONFIG_UNUSED_SYMBOLS
+#endif
+#ifdef CONFIG_UNUSED_SYMBOLS
+#endif
+#ifdef CONFIG_UNUSED_SYMBOLS
+#endif
+#ifdef CONFIG_UNUSED_SYMBOLS
+#endif
+#ifdef CONFIG_UNUSED_SYMBOLS
+#endif
+#ifdef CONFIG_UNUSED_SYMBOLS
+#endif
+#ifdef CONFIG_UNUSED_SYMBOLS
+#endif

is a bit of a maintenance problem though.  Can you think of a way in whcih
we can cut down on that?


  reply	other threads:[~2007-09-13 23:01 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-13 22:30 [PATCH] module.c: add missing ifdefs for CONFIG_UNUSED_SYMBOLS Denys Vlasenko
2007-09-13 23:00 ` Andrew Morton [this message]
2007-09-13 23:23   ` Denys Vlasenko

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=20070913160047.159e5e3d.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vda.linux@googlemail.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