Re: [PATCH 4/7] module: avoid code duplication in include/linux/export.h

From: Matthias Maennich
Date: Fri Sep 27 2019 - 08:36:23 EST


On Fri, Sep 27, 2019 at 01:07:33PM +0200, Rasmus Villemoes wrote:
On 27/09/2019 11.36, Masahiro Yamada wrote:
include/linux/export.h has lots of code duplication between
EXPORT_SYMBOL and EXPORT_SYMBOL_NS.

To improve the maintainability and readability, unify the
implementation.

When the symbol has no namespace, pass the empty string "" to
the 'ns' parameter.

The drawback of this change is, it grows the code size.
When the symbol has no namespace, sym->namespace was previously
NULL, but it is now am empty string "". So, it increases 1 byte
for every no namespace EXPORT_SYMBOL.

A typical kernel configuration has 10K exported symbols, so it
increases 10KB in rough estimation.

I did not come up with a good idea to refactor it without increasing
the code size.

Can't we put the "aMS" flags on the __ksymtab_strings section? That
would make the empty strings free, and would also deduplicate the
USB_STORAGE string. And while almost per definition we don't have exact
duplicates among the names of exported symbols, we might have both a foo
and __foo, so that could save even more.

I was not aware of this possibility and I was a bit bothered that I was
not able to deduplicate the namespace strings in the PREL case. So, at
least I tried to avoid having the redundant empty strings in it. If this
approach solves the deduplication problem _and_ reduces the complexity
of the code, I am very much for it. I will only be able to look into
this next week.

I don't know if we have it already, but we'd need each arch to tell us
what symbol to use for @ in @progbits (e.g. % for arm). It seems most
are fine with @, so maybe a generic version could be

#ifndef ARCH_SECTION_TYPE_CHAR
#define ARCH_SECTION_TYPE_CHAR "@"
#endif

and then it would be
section("__ksymtab_strings,\"aMS\","ARCH_SECTION_TYPE_CHAR"progbits,1")

But I don't know if any tooling relies on the strings not being
deduplicated.

Matthias
Cheers,

Rasmus