[RFC PATCH 0/3] Add kconfig symbol as module attribute

[RFC PATCH 0/3] Add kconfig symbol as module attribute

[Cristina Moraru]

This patchset implements dynamic pegging of kconfig symbol
into driver modinfo section

* adds a kconfig symbol attribute to struct module
* updates streamline_config.pl to generate the auxiliary file
scripts/mod/Module.ksymb containing associations of driver file
names and corresponding kconfig symbols CONFIG_*
* updates modpost to use the information from Module.ksymb to
add the content of the attribute kconfig_symbol.

Please note that this patchset is part of a research and
currently does not provide complete correctness or efficiency.

The result of this patchset is the following: the attribute
kconfig_symbol is added but only for some modules, namely for
those for which the module name corresponds to the source file
name of the driver. This has been observed by the fact that
there are more srcversion attributes than kconfig_symbol. This
happens mostly because, in some cases, the driver name does not
match the registered module name (more details in the the PATCH2
commit message). Also, in file Module.ksymb some object names
have more than one CONFIG_* symbol. This is because that object
it may be a platform independent component that is linked to more
than one driver. So, all CONFIGs in which is found appear as
associated with this object. However, I'm guessing this doesn't
happen for final individual modules.
Currently, for the sake of the proof of concept, the first of
the CONFIG_* options is considered.

Usage:
First run 'make localmodconfig' in order to generate the .config
file and Module.ksymb file with the associations. Then compile
the kernel. In the machine booted with this kernel kconfig_symbol
attributes should appear in /sys instances of the modules.

Thanks,
Cristina

Cristina Moraru (3):
  Add kconfig_symbol attribute to struct module
  Add generation of Module.ksymb file in streamline_config.pl
  Add dynamic pegging of Kconfig symbol

 include/linux/module.h               |  1 +
 kernel/module.c                      |  1 +
 scripts/kconfig/streamline_config.pl | 20 +++++++++++++++
 scripts/mod/modpost.c                | 47 ++++++++++++++++++++++++++++++++++++
 scripts/mod/modpost.h                |  1 +
 5 files changed, 70 insertions(+)

-- 
2.7.4

[RFC PATCH 1/3] Add kconfig_symbol attribute to struct module

[Cristina Moraru]

Create additional attribute in struct module
in order for each module to store its associate
kconfig CONFIG_* symbol.

The goal is to enable each module to expose in
/sys its corresponding CONFIG_* option. The value
of this attribute will be dynamically pegged by
modpost without requiring extra work from the
driver developers. Further, this information will
be used by a hardware interogation tool to extract
build information about the existent devices.

This patch is part of a research project within
Google Summer of Code of porting 'make localmodconfig'
for backported drivers.

Signed-off-by: Cristina Moraru <cristina.moraru09@gmail.com>
---
 include/linux/module.h | 1 +
 kernel/module.c        | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/linux/module.h b/include/linux/module.h
index 3daf2b3..bef5e44 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -353,6 +353,7 @@ struct module {
 	struct module_attribute *modinfo_attrs;
 	const char *version;
 	const char *srcversion;
+	const char *kconfig_symbol;
 	struct kobject *holders_dir;
 
 	/* Exported symbols */
diff --git a/kernel/module.c b/kernel/module.c
index 5f71aa6..4463c6c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -757,6 +757,7 @@ static struct module_attribute modinfo_##field = {                    \
 
 MODINFO_ATTR(version);
 MODINFO_ATTR(srcversion);
+MODINFO_ATTR(kconfig_symbol);
 
 static char last_unloaded_module[MODULE_NAME_LEN+1];
 
-- 
2.7.4

[RFC PATCH 2/3] Add generation of Module.ksymb file in streamline_config.pl

[Cristina Moraru]

Add generation of ./scripts/mod/Module.ksymb file containing
associations of driver file names and corresponding CONFIG_*
symbol.

This file will be used by modpost to peg kconfig CONFIG_*
symbol to its corresponding module. This information will
be further exposed in userspace for extracting build options
for the required modules.

This approach faces the following limitations:
* in some cases there are more than one CONFIG_* option
for certain objects. This happens for the objects that are
part of more CONFIGs. Thus, all configs are returned for
this object names. For example, the mapping for clk_div6 is
CONFIG_ARCH_R8A73A4, CONFIG_ARCH_R8A7793 and many others.
* in some cases the driver file name does not match the
registered name for the module. For example:

Driver filename		Module name
-----------------------------------
lineage-pem[.o]		lineage_pem
phy-ab8500-usb[.o]	abx5x0-usb
ehci-mxc[.o]		mxc-ehci
etc.

There is no naming rule / standard between the driver
name and the registered module name.

This patch is part of a research project within
Google Summer of Code of porting 'make localmodconfig'
for backported drivers.

Signed-off-by: Cristina Moraru <cristina.moraru09@gmail.com>
---
 scripts/kconfig/streamline_config.pl | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index b8c7b29..4833ede 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -147,6 +147,7 @@ my %objects;
 my $var;
 my $iflevel = 0;
 my @ifdeps;
+my @drv_objs;
 
 # prevent recursion
 my %read_kconfigs;
@@ -341,6 +342,10 @@ foreach my $makefile (@makefiles) {
 		    # The objects have a hash mapping to a reference
 		    # of an array of configs.
 		    $objects{$1} = \@arr;
+		    # Save objects corresponding to driver Makefiles
+		    if (index($makefile, "./drivers/") == 0) {
+			    push(@drv_objs, substr($obj, 0, -2));
+			}
 		}
 	    }
 	}
@@ -348,6 +353,21 @@ foreach my $makefile (@makefiles) {
     close($infile);
 }
 
+sub gen_module_kconfigs {
+
+	my $module_ksymb = $ENV{'objtree'}."/scripts/mod/Module.ksymb";
+	my $key;
+
+	open(my $ksymbfile, '>', $module_ksymb) || die "Can not open $module_ksymb for writing";
+
+	foreach (@drv_objs) {
+		print $ksymbfile "$_ " . "@{$objects{$_}}\n";
+	}
+	close $ksymbfile;
+}
+
+gen_module_kconfigs();
+
 my %modules;
 my $linfile;
 
-- 
2.7.4

[RFC PATCH 3/3] Add dynamic pegging of Kconfig symbol

[Cristina Moraru]

Update modpost to add dynamic pegging of CONFIG_* symbol
from file ./scripts/mod/Module.ksymb into kconfig_symbol
attribute of struct module. This information will be
further exposed in userspace for extracting build options
for the required modules.

Note: this patch is part of a proof of concept and does
not represent the final version.

This patch is part of a research project within
Google Summer of Code of porting 'make localmodconfig'
for backported drivers.

Signed-off-by: Cristina Moraru <cristina.moraru09@gmail.com>
---
 scripts/mod/modpost.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 scripts/mod/modpost.h |  1 +
 2 files changed, 48 insertions(+)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 48958d3..d80c062 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -42,6 +42,8 @@ static int sec_mismatch_fatal = 0;
 /* ignore missing files */
 static int ignore_missing_files;
 
+#define MOD_KSYMB_FILENAME "scripts/mod/Module.ksymb"
+
 enum export {
 	export_plain,      export_unused,     export_gpl,
 	export_unused_gpl, export_gpl_future, export_unknown
@@ -2245,6 +2247,50 @@ static void add_srcversion(struct buffer *b, struct module *mod)
 	}
 }
 
+static void get_kconfig_symbol(struct module *mod) {
+
+	unsigned long size, pos = 0;
+	void *file = grab_file(MOD_KSYMB_FILENAME, &size);
+	char *line, *short_name;
+
+	if (!file)
+		return;
+
+	short_name = strrchr(mod->name, '/');
+	short_name++;
+
+	while ((line = get_next_line(&pos, file, size))) {
+		char *modname, *kconfig_symbol, *p;
+
+		modname = line;
+		if (!(p = strchr(line, ' ')))
+			goto fail;
+		*p++ = '\0';
+		if (!strcmp(short_name, modname)) {
+			kconfig_symbol = p;
+			if ((p = strchr(kconfig_symbol, ' ')))
+				*p = '\0';
+			strcpy(mod->kconfig_symbol, kconfig_symbol);
+			break;
+		}
+	}
+	release_file(file, size);
+	return;
+fail:
+	release_file(file, size);
+	fatal("parse error in Module.ksymb file\n");
+}
+
+static void add_kconfig_symbol(struct buffer *b, struct module *mod)
+{
+	get_kconfig_symbol(mod);
+	if (mod->kconfig_symbol[0]) {
+		buf_printf(b, "\n");
+		buf_printf(b, "MODULE_INFO(kconfig_symbol, \"%s\");\n",
+			   mod->kconfig_symbol);
+	}
+}
+
 static void write_if_changed(struct buffer *b, const char *fname)
 {
 	char *tmp;
@@ -2478,6 +2524,7 @@ int main(int argc, char **argv)
 		add_depends(&buf, mod, modules);
 		add_moddevtable(&buf, mod);
 		add_srcversion(&buf, mod);
+		add_kconfig_symbol(&buf, mod);
 
 		sprintf(fname, "%s.mod.c", mod->name);
 		write_if_changed(&buf, fname);
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 6a5e151..1ba48b1 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -119,6 +119,7 @@ struct module {
 	int has_cleanup;
 	struct buffer dev_table_buf;
 	char	     srcversion[25];
+	char	     kconfig_symbol[50];
 	int is_dot_o;
 };
 
-- 
2.7.4

Re: [RFC PATCH 0/3] Add kconfig symbol as module attribute

[Luis R. Rodriguez]

On Sun, Jul 31, 2016 at 05:33:49PM +0200, Cristina Moraru wrote:
> This patchset implements dynamic pegging of kconfig symbol
> into driver modinfo section
> 
> * adds a kconfig symbol attribute to struct module
> * updates streamline_config.pl to generate the auxiliary file
> scripts/mod/Module.ksymb containing associations of driver file
> names and corresponding kconfig symbols CONFIG_*
> * updates modpost to use the information from Module.ksymb to
> add the content of the attribute kconfig_symbol.
> 
> Please note that this patchset is part of a research and
> currently does not provide complete correctness or efficiency.
> 
> The result of this patchset is the following: the attribute
> kconfig_symbol is added but only for some modules, namely for
> those for which the module name corresponds to the source file
> name of the driver. This has been observed by the fact that
> there are more srcversion attributes than kconfig_symbol. This
> happens mostly because, in some cases, the driver name does not
> match the registered module name (more details in the the PATCH2
> commit message). Also, in file Module.ksymb some object names
> have more than one CONFIG_* symbol. This is because that object
> it may be a platform independent component that is linked to more
> than one driver. So, all CONFIGs in which is found appear as
> associated with this object. However, I'm guessing this doesn't
> happen for final individual modules.
> Currently, for the sake of the proof of concept, the first of
> the CONFIG_* options is considered.
> 
> Usage:
> First run 'make localmodconfig' in order to generate the .config
> file and Module.ksymb file with the associations. Then compile
> the kernel. In the machine booted with this kernel kconfig_symbol
> attributes should appear in /sys instances of the modules.

Thanks Cristina, it helps to explain the motivation. Can you help describe
that? In subsequent patch iterations please include that? I'll go review the
patches now!

  Luis

Re: [RFC PATCH 2/3] Add generation of Module.ksymb file in streamline_config.pl

[Luis R. Rodriguez]

On Sun, Jul 31, 2016 at 05:33:51PM +0200, Cristina Moraru wrote:
> Add generation of ./scripts/mod/Module.ksymb file containing
> associations of driver file names and corresponding CONFIG_*
> symbol.
> 
> This file will be used by modpost to peg kconfig CONFIG_*
> symbol to its corresponding module. This information will
> be further exposed in userspace for extracting build options
> for the required modules.
> 
> This approach faces the following limitations:
> * in some cases there are more than one CONFIG_* option
> for certain objects. This happens for the objects that are
> part of more CONFIGs. Thus, all configs are returned for
> this object names. For example, the mapping for clk_div6 is
> CONFIG_ARCH_R8A73A4, CONFIG_ARCH_R8A7793 and many others.

Ah, indeed so for instance:

drivers/clk/renesas/Makefile:
...
obj-$(CONFIG_ARCH_R8A73A4)              += clk-r8a73a4.o clk-div6.o             
obj-$(CONFIG_ARCH_R8A7740)              += clk-r8a7740.o clk-div6.o  
...

So in this case there is no particular unique CONFIG_* symbols that
only associates itself to clk-div6.

Given that the purpose here is to help compile a .config that is sufficient to
build a kernel with that module, I do believe using both config symbols would
be the appropriate solution in this case to ensure a build suffices based only
on this information.  This is only possible of course *iff* both symbols are
not mutually exclusive, so in this case an issue would be if for instance
CONFIG_ARCH_R8A73A4's kconfig entry negates CONFIG_ARCH_R8A7740. They do not
in this case so using both suffices. I can imagine doing this secondary logic
is cumbersome, so perhaps its best we avoid these sorts of situations as it
would imply doing more work going barkwards -- from modules loaded to modules
to symbols.

I'd bet this would not be the only kconfig issue that could arise from this
loose practice in kconfig.

Anyway, if we determine that both kconfig options should be enabled for a build
to select this driver -- that would increase the build size, perhaps with no
need for it. So this strategy of course would not yield optimal builds. So we
should just expand the kconfig documentation indicating pitfalls of this use
of kconfig, given the deterministic gains we want of mapping between modules
to config symbols.

It'd be curious to see metrically how many symbols we have like this which are
modules and if there is a benefit to add a respective first class config symbol
to each. Is that too much to ask to hunt quantitatively ? Based on this we can
determine if a tree-wide cleaning is in order. If folks already know this is
outright dumb please let me know. Additionally -- are there other areas in the
kernel that suffer from the lack of direct deterministic relationship between
modules and a kconfig symbol ? Or how about the other way around: is there any
valid hard requirement to enable this loose practice on the kernel ? If we
remove this loose semantic, do we loose anything ? So far I see only gains:

 o Once you have modules loaded, have the ability to do a deterministic
   set of .config symbol entries you know you need for an optimal smaller build
   for your device. If you have a direct mapping a tool that scrapes this
   list can be kconfig language stupid, and does not need to inspect your other
   symbols or upstream Kconfig entry for issues with other symbols

 o Having a direct mapping means selecting only the real configuration options
   needed

 o Forcing a bit of documenting (through the configuration menu) more clearly
   what this module does

> * in some cases the driver file name does not match the
> registered name for the module. For example:
> 
> Driver filename		Module name
> -----------------------------------
> lineage-pem[.o]		lineage_pem
>
> phy-ab8500-usb[.o]	abx5x0-usb
>
> ehci-mxc[.o]		mxc-ehci
> etc.

To be clear you mean here for instance:

drivers/usb/phy/phy-ab8500-usb.c
...
static struct platform_driver ab8500_usb_driver = {                             
        .probe          = ab8500_usb_probe,                                     
        .remove         = ab8500_usb_remove,                                    
        .id_table       = ab8500_usb_devtype,                                   
        .driver         = {                                                     
                .name   = "abx5x0-usb",                                         
        },                                                                      
};  
...

The .ko filename should I think typically match the respective principle
object filename, would that help? So for instance while the name above
is abx5x0-usb, the ko file should still be drivers/usb/phy/phy-ab8500-usb.ko

There may be some exceptions to this but this probably is not that common?

> There is no naming rule / standard between the driver
> name and the registered module name.

This seems true. In this case the module name should match the
target object typically.

> This patch is part of a research project within
> Google Summer of Code of porting 'make localmodconfig'
> for backported drivers.
> 
> Signed-off-by: Cristina Moraru <cristina.moraru09@gmail.com>
> ---
>  scripts/kconfig/streamline_config.pl | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index b8c7b29..4833ede 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -147,6 +147,7 @@ my %objects;
>  my $var;
>  my $iflevel = 0;
>  my @ifdeps;
> +my @drv_objs;
>  
>  # prevent recursion
>  my %read_kconfigs;
> @@ -341,6 +342,10 @@ foreach my $makefile (@makefiles) {
>  		    # The objects have a hash mapping to a reference
>  		    # of an array of configs.
>  		    $objects{$1} = \@arr;
> +		    # Save objects corresponding to driver Makefiles
> +		    if (index($makefile, "./drivers/") == 0) {
> +			    push(@drv_objs, substr($obj, 0, -2));
> +			}
>  		}
>  	    }
>  	}
> @@ -348,6 +353,21 @@ foreach my $makefile (@makefiles) {
>      close($infile);
>  }
>  
> +sub gen_module_kconfigs {
> +
> +	my $module_ksymb = $ENV{'objtree'}."/scripts/mod/Module.ksymb";
> +	my $key;
> +
> +	open(my $ksymbfile, '>', $module_ksymb) || die "Can not open $module_ksymb for writing";
> +
> +	foreach (@drv_objs) {
> +		print $ksymbfile "$_ " . "@{$objects{$_}}\n";
> +	}
> +	close $ksymbfile;
> +}
> +
> +gen_module_kconfigs();
> +
>  my %modules;
>  my $linfile;

It would be good to see how we could not rely on scripts/kconfig/streamline_config.pl
for this purpose. That should only be used for helping with localmodconfig. In this
case we want to make the kconfig symbol available to all modules when possible, so
we do not want to depend on scripts/kconfig/streamline_config.pl generically.

  Luis

Re: [RFC PATCH 3/3] Add dynamic pegging of Kconfig symbol

[Luis R. Rodriguez]

On Sun, Jul 31, 2016 at 05:33:52PM +0200, Cristina Moraru wrote:
> Update modpost to add dynamic pegging of CONFIG_* symbol
> from file ./scripts/mod/Module.ksymb into kconfig_symbol
> attribute of struct module. This information will be
> further exposed in userspace for extracting build options
> for the required modules.
> 
> Note: this patch is part of a proof of concept and does
> not represent the final version.
> 
> This patch is part of a research project within
> Google Summer of Code of porting 'make localmodconfig'
> for backported drivers.
> 
> Signed-off-by: Cristina Moraru <cristina.moraru09@gmail.com>
> ---
>  scripts/mod/modpost.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  scripts/mod/modpost.h |  1 +
>  2 files changed, 48 insertions(+)
> 
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 48958d3..d80c062 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -42,6 +42,8 @@ static int sec_mismatch_fatal = 0;
>  /* ignore missing files */
>  static int ignore_missing_files;
>  
> +#define MOD_KSYMB_FILENAME "scripts/mod/Module.ksymb"
> +
>  enum export {
>  	export_plain,      export_unused,     export_gpl,
>  	export_unused_gpl, export_gpl_future, export_unknown
> @@ -2245,6 +2247,50 @@ static void add_srcversion(struct buffer *b, struct module *mod)
>  	}
>  }
>  
> +static void get_kconfig_symbol(struct module *mod) {
> +
> +	unsigned long size, pos = 0;
> +	void *file = grab_file(MOD_KSYMB_FILENAME, &size);
> +	char *line, *short_name;
> +
> +	if (!file)
> +		return;
> +
> +	short_name = strrchr(mod->name, '/');
> +	short_name++;
> +
> +	while ((line = get_next_line(&pos, file, size))) {
> +		char *modname, *kconfig_symbol, *p;
> +
> +		modname = line;
> +		if (!(p = strchr(line, ' ')))
> +			goto fail;
> +		*p++ = '\0';
> +		if (!strcmp(short_name, modname)) {

You had mentioned in the last patch that mod->name seemed to
get the respective name used in the struct driver, however
I am not seeing that. At least modpost seems to do this:

MODLISTCMD := find $(MODVERDIR) -name '*.mod' | xargs -r grep -h '\.ko$$' | sort -u

The extra $ is needed since this is part of for the makefile, to run this yourself
after a build do:

find ./ -name '*.mod' | xargs -r grep -h '\.ko$' | sort -u

This is the first step. It hunts for all modules.

Second step is:

cmd_modpost = $(MODLISTCMD) | sed 's/\.ko$$/.o/' | $(modpost) $(MODPOST_OPT) -s -T -

The sed there just swaps the .ko for .o  and then it runs modpost, taking the filenames
from stdin. We help modpost further to highlight that the module list is in stdin by
it using "-T -", this calls read_symbols_from_files("-"). From this it iterates over
each module list and for each it calls read_symbols(). The name is set via

 mod = new_module();

This just strips the .o but the same object build name is used. Perhaps the issue
is more of where the userspace tool you are using is getting the driver name instead?
If that's the issue the tool I think provides still the file path and as such the
object name, so that could be used instead ?

> +			kconfig_symbol = p;
> +			if ((p = strchr(kconfig_symbol, ' ')))
> +				*p = '\0';
> +			strcpy(mod->kconfig_symbol, kconfig_symbol);
> +			break;
> +		}
> +	}
> +	release_file(file, size);
> +	return;
> +fail:
> +	release_file(file, size);
> +	fatal("parse error in Module.ksymb file\n");
> +}
> +
> +static void add_kconfig_symbol(struct buffer *b, struct module *mod)
> +{
> +	get_kconfig_symbol(mod);
> +	if (mod->kconfig_symbol[0]) {
> +		buf_printf(b, "\n");
> +		buf_printf(b, "MODULE_INFO(kconfig_symbol, \"%s\");\n",
> +			   mod->kconfig_symbol);
> +	}
> +}
> +
>  static void write_if_changed(struct buffer *b, const char *fname)
>  {
>  	char *tmp;
> @@ -2478,6 +2524,7 @@ int main(int argc, char **argv)
>  		add_depends(&buf, mod, modules);
>  		add_moddevtable(&buf, mod);
>  		add_srcversion(&buf, mod);
> +		add_kconfig_symbol(&buf, mod);
>  
>  		sprintf(fname, "%s.mod.c", mod->name);
>  		write_if_changed(&buf, fname);
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 6a5e151..1ba48b1 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -119,6 +119,7 @@ struct module {
>  	int has_cleanup;
>  	struct buffer dev_table_buf;
>  	char	     srcversion[25];
> +	char	     kconfig_symbol[50];

Other than that please check for the longest symbol name here, is 50 enough?
Can we add a build warning / compile issue if this is longer than 50 on modpost?
That would in turn avoid possible bugs, and we'd be setting a limit. But really
is 50 the limit we want ?

  Luis

Re: [RFC PATCH 2/3] Add generation of Module.ksymb file in streamline_config.pl

[Julia Lawall]

On Mon, 8 Aug 2016, Luis R. Rodriguez wrote:

> On Sun, Jul 31, 2016 at 05:33:51PM +0200, Cristina Moraru wrote:
> > Add generation of ./scripts/mod/Module.ksymb file containing
> > associations of driver file names and corresponding CONFIG_*
> > symbol.
> >
> > This file will be used by modpost to peg kconfig CONFIG_*
> > symbol to its corresponding module. This information will
> > be further exposed in userspace for extracting build options
> > for the required modules.
> >
> > This approach faces the following limitations:
> > * in some cases there are more than one CONFIG_* option
> > for certain objects. This happens for the objects that are
> > part of more CONFIGs. Thus, all configs are returned for
> > this object names. For example, the mapping for clk_div6 is
> > CONFIG_ARCH_R8A73A4, CONFIG_ARCH_R8A7793 and many others.
>
> Ah, indeed so for instance:
>
> drivers/clk/renesas/Makefile:
> ...
> obj-$(CONFIG_ARCH_R8A73A4)              += clk-r8a73a4.o clk-div6.o
> obj-$(CONFIG_ARCH_R8A7740)              += clk-r8a7740.o clk-div6.o
> ...
>
> So in this case there is no particular unique CONFIG_* symbols that
> only associates itself to clk-div6.
>
> Given that the purpose here is to help compile a .config that is sufficient to
> build a kernel with that module, I do believe using both config symbols would
> be the appropriate solution in this case to ensure a build suffices based only
> on this information.  This is only possible of course *iff* both symbols are
> not mutually exclusive, so in this case an issue would be if for instance
> CONFIG_ARCH_R8A73A4's kconfig entry negates CONFIG_ARCH_R8A7740. They do not
> in this case so using both suffices. I can imagine doing this secondary logic
> is cumbersome, so perhaps its best we avoid these sorts of situations as it
> would imply doing more work going barkwards -- from modules loaded to modules
> to symbols.
>
> I'd bet this would not be the only kconfig issue that could arise from this
> loose practice in kconfig.
>
> Anyway, if we determine that both kconfig options should be enabled for a build
> to select this driver -- that would increase the build size, perhaps with no
> need for it. So this strategy of course would not yield optimal builds.

Do you care?  I guess no one would want clk-div6 for actual execution.  I
haven't looked at the file, but from the make information, it looks like a
library that is shared by two drivers and has no independent interest.  If
the goal is just to be sure that the code is compiled, for sanity checking
purposes, then wouldn't it be fine to either pick one option, or pick both
(giving perhaps a little more confidence at a small cost).

julia

> So we
> should just expand the kconfig documentation indicating pitfalls of this use
> of kconfig, given the deterministic gains we want of mapping between modules
> to config symbols.
>
> It'd be curious to see metrically how many symbols we have like this which are
> modules and if there is a benefit to add a respective first class config symbol
> to each. Is that too much to ask to hunt quantitatively ? Based on this we can
> determine if a tree-wide cleaning is in order. If folks already know this is
> outright dumb please let me know. Additionally -- are there other areas in the
> kernel that suffer from the lack of direct deterministic relationship between
> modules and a kconfig symbol ? Or how about the other way around: is there any
> valid hard requirement to enable this loose practice on the kernel ? If we
> remove this loose semantic, do we loose anything ? So far I see only gains:
>
>  o Once you have modules loaded, have the ability to do a deterministic
>    set of .config symbol entries you know you need for an optimal smaller build
>    for your device. If you have a direct mapping a tool that scrapes this
>    list can be kconfig language stupid, and does not need to inspect your other
>    symbols or upstream Kconfig entry for issues with other symbols
>
>  o Having a direct mapping means selecting only the real configuration options
>    needed
>
>  o Forcing a bit of documenting (through the configuration menu) more clearly
>    what this module does
>
> > * in some cases the driver file name does not match the
> > registered name for the module. For example:
> >
> > Driver filename		Module name
> > -----------------------------------
> > lineage-pem[.o]		lineage_pem
> >
> > phy-ab8500-usb[.o]	abx5x0-usb
> >
> > ehci-mxc[.o]		mxc-ehci
> > etc.
>
> To be clear you mean here for instance:
>
> drivers/usb/phy/phy-ab8500-usb.c
> ...
> static struct platform_driver ab8500_usb_driver = {
>         .probe          = ab8500_usb_probe,
>         .remove         = ab8500_usb_remove,
>         .id_table       = ab8500_usb_devtype,
>         .driver         = {
>                 .name   = "abx5x0-usb",
>         },
> };
> ...
>
> The .ko filename should I think typically match the respective principle
> object filename, would that help? So for instance while the name above
> is abx5x0-usb, the ko file should still be drivers/usb/phy/phy-ab8500-usb.ko
>
> There may be some exceptions to this but this probably is not that common?
>
> > There is no naming rule / standard between the driver
> > name and the registered module name.
>
> This seems true. In this case the module name should match the
> target object typically.
>
> > This patch is part of a research project within
> > Google Summer of Code of porting 'make localmodconfig'
> > for backported drivers.
> >
> > Signed-off-by: Cristina Moraru <cristina.moraru09@gmail.com>
> > ---
> >  scripts/kconfig/streamline_config.pl | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> > index b8c7b29..4833ede 100755
> > --- a/scripts/kconfig/streamline_config.pl
> > +++ b/scripts/kconfig/streamline_config.pl
> > @@ -147,6 +147,7 @@ my %objects;
> >  my $var;
> >  my $iflevel = 0;
> >  my @ifdeps;
> > +my @drv_objs;
> >
> >  # prevent recursion
> >  my %read_kconfigs;
> > @@ -341,6 +342,10 @@ foreach my $makefile (@makefiles) {
> >  		    # The objects have a hash mapping to a reference
> >  		    # of an array of configs.
> >  		    $objects{$1} = \@arr;
> > +		    # Save objects corresponding to driver Makefiles
> > +		    if (index($makefile, "./drivers/") == 0) {
> > +			    push(@drv_objs, substr($obj, 0, -2));
> > +			}
> >  		}
> >  	    }
> >  	}
> > @@ -348,6 +353,21 @@ foreach my $makefile (@makefiles) {
> >      close($infile);
> >  }
> >
> > +sub gen_module_kconfigs {
> > +
> > +	my $module_ksymb = $ENV{'objtree'}."/scripts/mod/Module.ksymb";
> > +	my $key;
> > +
> > +	open(my $ksymbfile, '>', $module_ksymb) || die "Can not open $module_ksymb for writing";
> > +
> > +	foreach (@drv_objs) {
> > +		print $ksymbfile "$_ " . "@{$objects{$_}}\n";
> > +	}
> > +	close $ksymbfile;
> > +}
> > +
> > +gen_module_kconfigs();
> > +
> >  my %modules;
> >  my $linfile;
>
> It would be good to see how we could not rely on scripts/kconfig/streamline_config.pl
> for this purpose. That should only be used for helping with localmodconfig. In this
> case we want to make the kconfig symbol available to all modules when possible, so
> we do not want to depend on scripts/kconfig/streamline_config.pl generically.
>
>   Luis
>

Re: [RFC PATCH 2/3] Add generation of Module.ksymb file in streamline_config.pl

[Luis R. Rodriguez]

On Mon, Aug 08, 2016 at 10:32:46PM +0200, Julia Lawall wrote:
> 
> 
> On Mon, 8 Aug 2016, Luis R. Rodriguez wrote:
> 
> > On Sun, Jul 31, 2016 at 05:33:51PM +0200, Cristina Moraru wrote:
> > > Add generation of ./scripts/mod/Module.ksymb file containing
> > > associations of driver file names and corresponding CONFIG_*
> > > symbol.
> > >
> > > This file will be used by modpost to peg kconfig CONFIG_*
> > > symbol to its corresponding module. This information will
> > > be further exposed in userspace for extracting build options
> > > for the required modules.
> > >
> > > This approach faces the following limitations:
> > > * in some cases there are more than one CONFIG_* option
> > > for certain objects. This happens for the objects that are
> > > part of more CONFIGs. Thus, all configs are returned for
> > > this object names. For example, the mapping for clk_div6 is
> > > CONFIG_ARCH_R8A73A4, CONFIG_ARCH_R8A7793 and many others.
> >
> > Ah, indeed so for instance:
> >
> > drivers/clk/renesas/Makefile:
> > ...
> > obj-$(CONFIG_ARCH_R8A73A4)              += clk-r8a73a4.o clk-div6.o
> > obj-$(CONFIG_ARCH_R8A7740)              += clk-r8a7740.o clk-div6.o
> > ...
> >
> > So in this case there is no particular unique CONFIG_* symbols that
> > only associates itself to clk-div6.
> >
> > Given that the purpose here is to help compile a .config that is sufficient to
> > build a kernel with that module, I do believe using both config symbols would
> > be the appropriate solution in this case to ensure a build suffices based only
> > on this information.  This is only possible of course *iff* both symbols are
> > not mutually exclusive, so in this case an issue would be if for instance
> > CONFIG_ARCH_R8A73A4's kconfig entry negates CONFIG_ARCH_R8A7740. They do not
> > in this case so using both suffices. I can imagine doing this secondary logic
> > is cumbersome, so perhaps its best we avoid these sorts of situations as it
> > would imply doing more work going barkwards -- from modules loaded to modules
> > to symbols.
> >
> > I'd bet this would not be the only kconfig issue that could arise from this
> > loose practice in kconfig.
> >
> > Anyway, if we determine that both kconfig options should be enabled for a build
> > to select this driver -- that would increase the build size, perhaps with no
> > need for it. So this strategy of course would not yield optimal builds.
> 
> Do you care?  I guess no one would want clk-div6 for actual execution.  I
> haven't looked at the file, but from the make information, it looks like a
> library that is shared by two drivers and has no independent interest.  If
> the goal is just to be sure that the code is compiled, for sanity checking
> purposes, then wouldn't it be fine to either pick one option, or pick both
> (giving perhaps a little more confidence at a small cost).

Indeed but both kconfig options may be mutually exclusive, in such case a tool
trying to pick what should be enabled must do more work, maybe read some
Kconfig and then understand that language.

I'm a bit more inclined to close the gap and leave this ambiguity out of the
kconfig picture if possible so we have 1-1 mappings for modules at least, then
dependencies are explicit and tools doing backward mapping would not have to
learn kconfig.

  Luis

Re: [RFC PATCH 2/3] Add generation of Module.ksymb file in streamline_config.pl

[Geert Uytterhoeven]

On Mon, Aug 8, 2016 at 9:15 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Sun, Jul 31, 2016 at 05:33:51PM +0200, Cristina Moraru wrote:
>> Add generation of ./scripts/mod/Module.ksymb file containing
>> associations of driver file names and corresponding CONFIG_*
>> symbol.
>>
>> This file will be used by modpost to peg kconfig CONFIG_*
>> symbol to its corresponding module. This information will
>> be further exposed in userspace for extracting build options
>> for the required modules.
>>
>> This approach faces the following limitations:
>> * in some cases there are more than one CONFIG_* option
>> for certain objects. This happens for the objects that are
>> part of more CONFIGs. Thus, all configs are returned for
>> this object names. For example, the mapping for clk_div6 is
>> CONFIG_ARCH_R8A73A4, CONFIG_ARCH_R8A7793 and many others.
>
> Ah, indeed so for instance:
>
> drivers/clk/renesas/Makefile:
> ...
> obj-$(CONFIG_ARCH_R8A73A4)              += clk-r8a73a4.o clk-div6.o
> obj-$(CONFIG_ARCH_R8A7740)              += clk-r8a7740.o clk-div6.o
> ...
>
> So in this case there is no particular unique CONFIG_* symbols that
> only associates itself to clk-div6.
>
> Given that the purpose here is to help compile a .config that is sufficient to
> build a kernel with that module, I do believe using both config symbols would
> be the appropriate solution in this case to ensure a build suffices based only
> on this information.  This is only possible of course *iff* both symbols are
> not mutually exclusive, so in this case an issue would be if for instance
> CONFIG_ARCH_R8A73A4's kconfig entry negates CONFIG_ARCH_R8A7740. They do not
> in this case so using both suffices. I can imagine doing this secondary logic
> is cumbersome, so perhaps its best we avoid these sorts of situations as it
> would imply doing more work going barkwards -- from modules loaded to modules
> to symbols.
>
> I'd bet this would not be the only kconfig issue that could arise from this
> loose practice in kconfig.
>
> Anyway, if we determine that both kconfig options should be enabled for a build
> to select this driver -- that would increase the build size, perhaps with no
> need for it. So this strategy of course would not yield optimal builds. So we
> should just expand the kconfig documentation indicating pitfalls of this use
> of kconfig, given the deterministic gains we want of mapping between modules
> to config symbols.

If you would not only look at loaded modules, but also at the compatible
values in the DTB, you could find out which of the two symbols is needed.

E.g. on r8a7740, you would discover that you need clk-r8a7740, hence
enable CONFIG_ARCH_R8A7740, which also build clk-div.o.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds