2019-08-21 11:56:38

by Matthias Männich

[permalink] [raw]
Subject: [PATCH v3 10/11] RFC: usb-storage: export symbols in USB_STORAGE namespace

Modules using these symbols are required to explicitly import the
namespace. This patch was generated with the following steps and serves
as a reference to use the symbol namespace feature:

1) Define DEFAULT_SYMBOL_NAMESPACE in the corresponding Makefile
2) make (see warnings during modpost about missing imports)
3) make nsdeps

Instead of a DEFAULT_SYMBOL_NAMESPACE definition, the EXPORT_SYMBOL_NS
variants can be used to explicitly specify the namespace. The advantage
of the method used here is that newly added symbols are automatically
exported and existing ones are exported without touching their
respective EXPORT_SYMBOL macro expansion.

Signed-off-by: Matthias Maennich <[email protected]>
---
drivers/usb/storage/Makefile | 2 ++
drivers/usb/storage/alauda.c | 1 +
drivers/usb/storage/cypress_atacb.c | 1 +
drivers/usb/storage/datafab.c | 1 +
drivers/usb/storage/ene_ub6250.c | 1 +
drivers/usb/storage/freecom.c | 1 +
drivers/usb/storage/isd200.c | 1 +
drivers/usb/storage/jumpshot.c | 1 +
drivers/usb/storage/karma.c | 1 +
drivers/usb/storage/onetouch.c | 1 +
drivers/usb/storage/realtek_cr.c | 1 +
drivers/usb/storage/sddr09.c | 1 +
drivers/usb/storage/sddr55.c | 1 +
drivers/usb/storage/shuttle_usbat.c | 1 +
drivers/usb/storage/uas.c | 1 +
15 files changed, 16 insertions(+)

diff --git a/drivers/usb/storage/Makefile b/drivers/usb/storage/Makefile
index a67ddcbb4e24..46635fa4a340 100644
--- a/drivers/usb/storage/Makefile
+++ b/drivers/usb/storage/Makefile
@@ -8,6 +8,8 @@

ccflags-y := -I $(srctree)/drivers/scsi

+ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=USB_STORAGE
+
obj-$(CONFIG_USB_UAS) += uas.o
obj-$(CONFIG_USB_STORAGE) += usb-storage.o

diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c
index 6b8edf6178df..ddab2cd3d2e7 100644
--- a/drivers/usb/storage/alauda.c
+++ b/drivers/usb/storage/alauda.c
@@ -36,6 +36,7 @@
MODULE_DESCRIPTION("Driver for Alauda-based card readers");
MODULE_AUTHOR("Daniel Drake <[email protected]>");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(USB_STORAGE);

/*
* Status bytes
diff --git a/drivers/usb/storage/cypress_atacb.c b/drivers/usb/storage/cypress_atacb.c
index 4825902377eb..a6f3267bbef6 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -22,6 +22,7 @@
MODULE_DESCRIPTION("SAT support for Cypress USB/ATA bridges with ATACB");
MODULE_AUTHOR("Matthieu Castet <[email protected]>");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(USB_STORAGE);

/*
* The table of devices
diff --git a/drivers/usb/storage/datafab.c b/drivers/usb/storage/datafab.c
index 09353be199be..588818483f4b 100644
--- a/drivers/usb/storage/datafab.c
+++ b/drivers/usb/storage/datafab.c
@@ -54,6 +54,7 @@
MODULE_DESCRIPTION("Driver for Datafab USB Compact Flash reader");
MODULE_AUTHOR("Jimmie Mayfield <[email protected]>");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(USB_STORAGE);

struct datafab_info {
unsigned long sectors; /* total sector count */
diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
index c26129d5b943..8b1b73065421 100644
--- a/drivers/usb/storage/ene_ub6250.c
+++ b/drivers/usb/storage/ene_ub6250.c
@@ -26,6 +26,7 @@

MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(USB_STORAGE);
MODULE_FIRMWARE(SD_INIT1_FIRMWARE);
MODULE_FIRMWARE(SD_INIT2_FIRMWARE);
MODULE_FIRMWARE(SD_RW_FIRMWARE);
diff --git a/drivers/usb/storage/freecom.c b/drivers/usb/storage/freecom.c
index 4f542df37a44..34e7eaff1174 100644
--- a/drivers/usb/storage/freecom.c
+++ b/drivers/usb/storage/freecom.c
@@ -29,6 +29,7 @@
MODULE_DESCRIPTION("Driver for Freecom USB/IDE adaptor");
MODULE_AUTHOR("David Brown <[email protected]>");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(USB_STORAGE);

#ifdef CONFIG_USB_STORAGE_DEBUG
static void pdump(struct us_data *us, void *ibuffer, int length);
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 2b474d60b4db..c4da3fd6eff9 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -53,6 +53,7 @@
MODULE_DESCRIPTION("Driver for In-System Design, Inc. ISD200 ASIC");
MODULE_AUTHOR("Björn Stenberg <[email protected]>");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(USB_STORAGE);

static int isd200_Initialization(struct us_data *us);

diff --git a/drivers/usb/storage/jumpshot.c b/drivers/usb/storage/jumpshot.c
index 917f170c4124..229bf0c1afc9 100644
--- a/drivers/usb/storage/jumpshot.c
+++ b/drivers/usb/storage/jumpshot.c
@@ -51,6 +51,7 @@
MODULE_DESCRIPTION("Driver for Lexar \"Jumpshot\" Compact Flash reader");
MODULE_AUTHOR("Jimmie Mayfield <[email protected]>");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(USB_STORAGE);

/*
* The table of devices
diff --git a/drivers/usb/storage/karma.c b/drivers/usb/storage/karma.c
index 395cf8fb5870..05cec81dcd3f 100644
--- a/drivers/usb/storage/karma.c
+++ b/drivers/usb/storage/karma.c
@@ -23,6 +23,7 @@
MODULE_DESCRIPTION("Driver for Rio Karma");
MODULE_AUTHOR("Bob Copeland <[email protected]>, Keith Bennett <[email protected]>");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(USB_STORAGE);

#define RIO_PREFIX "RIOP\x00"
#define RIO_PREFIX_LEN 5
diff --git a/drivers/usb/storage/onetouch.c b/drivers/usb/storage/onetouch.c
index 39a5009a41a6..a989fe930e21 100644
--- a/drivers/usb/storage/onetouch.c
+++ b/drivers/usb/storage/onetouch.c
@@ -25,6 +25,7 @@
MODULE_DESCRIPTION("Maxtor USB OneTouch hard drive button driver");
MODULE_AUTHOR("Nick Sillik <[email protected]>");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(USB_STORAGE);

#define ONETOUCH_PKT_LEN 0x02
#define ONETOUCH_BUTTON KEY_PROG1
diff --git a/drivers/usb/storage/realtek_cr.c b/drivers/usb/storage/realtek_cr.c
index cc794e25a0b6..edbe419053d6 100644
--- a/drivers/usb/storage/realtek_cr.c
+++ b/drivers/usb/storage/realtek_cr.c
@@ -35,6 +35,7 @@
MODULE_DESCRIPTION("Driver for Realtek USB Card Reader");
MODULE_AUTHOR("wwang <[email protected]>");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(USB_STORAGE);

static int auto_delink_en = 1;
module_param(auto_delink_en, int, S_IRUGO | S_IWUSR);
diff --git a/drivers/usb/storage/sddr09.c b/drivers/usb/storage/sddr09.c
index bc9da736bdfc..51bcd4a43690 100644
--- a/drivers/usb/storage/sddr09.c
+++ b/drivers/usb/storage/sddr09.c
@@ -47,6 +47,7 @@
MODULE_DESCRIPTION("Driver for SanDisk SDDR-09 SmartMedia reader");
MODULE_AUTHOR("Andries Brouwer <[email protected]>, Robert Baruch <[email protected]>");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(USB_STORAGE);

static int usb_stor_sddr09_dpcm_init(struct us_data *us);
static int sddr09_transport(struct scsi_cmnd *srb, struct us_data *us);
diff --git a/drivers/usb/storage/sddr55.c b/drivers/usb/storage/sddr55.c
index b8527c55335b..ba955d65eb0e 100644
--- a/drivers/usb/storage/sddr55.c
+++ b/drivers/usb/storage/sddr55.c
@@ -29,6 +29,7 @@
MODULE_DESCRIPTION("Driver for SanDisk SDDR-55 SmartMedia reader");
MODULE_AUTHOR("Simon Munton");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(USB_STORAGE);

/*
* The table of devices
diff --git a/drivers/usb/storage/shuttle_usbat.c b/drivers/usb/storage/shuttle_usbat.c
index 854498e1012c..54aa1392c9ca 100644
--- a/drivers/usb/storage/shuttle_usbat.c
+++ b/drivers/usb/storage/shuttle_usbat.c
@@ -48,6 +48,7 @@
MODULE_DESCRIPTION("Driver for SCM Microsystems (a.k.a. Shuttle) USB-ATAPI cable");
MODULE_AUTHOR("Daniel Drake <[email protected]>, Robert Baruch <[email protected]>");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(USB_STORAGE);

/* Supported device types */
#define USBAT_DEV_HP8200 0x01
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 047c5922618f..bf80d6f81f58 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -1219,5 +1219,6 @@ static struct usb_driver uas_driver = {
module_usb_driver(uas_driver);

MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(USB_STORAGE);
MODULE_AUTHOR(
"Hans de Goede <[email protected]>, Matthew Wilcox and Sarah Sharp");
--
2.23.0.rc1.153.gdeed80330f-goog


2019-08-21 12:46:31

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v3 10/11] RFC: usb-storage: export symbols in USB_STORAGE namespace

On Wed, Aug 21, 2019 at 12:49:25PM +0100, Matthias Maennich wrote:
> Modules using these symbols are required to explicitly import the
> namespace. This patch was generated with the following steps and serves
> as a reference to use the symbol namespace feature:
>
> 1) Define DEFAULT_SYMBOL_NAMESPACE in the corresponding Makefile
> 2) make (see warnings during modpost about missing imports)
> 3) make nsdeps
>
> Instead of a DEFAULT_SYMBOL_NAMESPACE definition, the EXPORT_SYMBOL_NS
> variants can be used to explicitly specify the namespace. The advantage
> of the method used here is that newly added symbols are automatically
> exported and existing ones are exported without touching their
> respective EXPORT_SYMBOL macro expansion.
>
> Signed-off-by: Matthias Maennich <[email protected]>

This looks good to me. This can be included with the rest of this
series when/if it goes through the kbuild or module tree:

Reviewed-by: Greg Kroah-Hartman <[email protected]>

Actually, which tree will this be going through?

thanks,

greg k-h

2019-08-21 14:38:46

by Jessica Yu

[permalink] [raw]
Subject: Re: [PATCH v3 10/11] RFC: usb-storage: export symbols in USB_STORAGE namespace

+++ Greg KH [21/08/19 05:38 -0700]:
>On Wed, Aug 21, 2019 at 12:49:25PM +0100, Matthias Maennich wrote:
>> Modules using these symbols are required to explicitly import the
>> namespace. This patch was generated with the following steps and serves
>> as a reference to use the symbol namespace feature:
>>
>> 1) Define DEFAULT_SYMBOL_NAMESPACE in the corresponding Makefile
>> 2) make (see warnings during modpost about missing imports)
>> 3) make nsdeps
>>
>> Instead of a DEFAULT_SYMBOL_NAMESPACE definition, the EXPORT_SYMBOL_NS
>> variants can be used to explicitly specify the namespace. The advantage
>> of the method used here is that newly added symbols are automatically
>> exported and existing ones are exported without touching their
>> respective EXPORT_SYMBOL macro expansion.
>>
>> Signed-off-by: Matthias Maennich <[email protected]>
>
>This looks good to me. This can be included with the rest of this
>series when/if it goes through the kbuild or module tree:
>
>Reviewed-by: Greg Kroah-Hartman <[email protected]>
>
>Actually, which tree will this be going through?

I would be happy to take the patchset through the modules tree once it
collects the appropriate ack/reviewed-by's and once I get a chance to
sit down and review/test it next week :)

Thanks!

Jessica

2019-08-22 00:04:33

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v3 10/11] RFC: usb-storage: export symbols in USB_STORAGE namespace

On Wed, Aug 21, 2019 at 12:49:25PM +0100, Matthias Maennich wrote:
> Modules using these symbols are required to explicitly import the
> namespace. This patch was generated with the following steps and serves
> as a reference to use the symbol namespace feature:
>
> 1) Define DEFAULT_SYMBOL_NAMESPACE in the corresponding Makefile
> 2) make (see warnings during modpost about missing imports)
> 3) make nsdeps
>
> Instead of a DEFAULT_SYMBOL_NAMESPACE definition, the EXPORT_SYMBOL_NS
> variants can be used to explicitly specify the namespace. The advantage
> of the method used here is that newly added symbols are automatically
> exported and existing ones are exported without touching their
> respective EXPORT_SYMBOL macro expansion.

So what is USB_STORAGE here? It isn't a C string, so where does it
come from? To me using a C string would seem like the nicer interface
vs a random cpp symbol that gets injected somewhere.

2019-08-22 09:38:57

by Matthias Männich

[permalink] [raw]
Subject: Re: [PATCH v3 10/11] RFC: usb-storage: export symbols in USB_STORAGE namespace

On Wed, Aug 21, 2019 at 04:13:29PM -0700, Christoph Hellwig wrote:
>On Wed, Aug 21, 2019 at 12:49:25PM +0100, Matthias Maennich wrote:
>> Modules using these symbols are required to explicitly import the
>> namespace. This patch was generated with the following steps and serves
>> as a reference to use the symbol namespace feature:
>>
>> 1) Define DEFAULT_SYMBOL_NAMESPACE in the corresponding Makefile
>> 2) make (see warnings during modpost about missing imports)
>> 3) make nsdeps
>>
>> Instead of a DEFAULT_SYMBOL_NAMESPACE definition, the EXPORT_SYMBOL_NS
>> variants can be used to explicitly specify the namespace. The advantage
>> of the method used here is that newly added symbols are automatically
>> exported and existing ones are exported without touching their
>> respective EXPORT_SYMBOL macro expansion.
>
>So what is USB_STORAGE here? It isn't a C string, so where does it
>come from? To me using a C string would seem like the nicer interface
>vs a random cpp symbol that gets injected somewhere.

To be honest, I would also prefer an interface that uses C strings or
literals for the new EXPORT_SYMBOLS* macros:

EXPORT_SYMBOL_NS(mysym, "USB_STORAGE");

or

const char USB_STORAGE_NS[] = "USB_STORAGE";
EXPORT_SYMBOL_NS(mysym, USB_STORAGE_NS);

The DEFAULT_SYMBOL_NAMESPACE define within Makefiles would get a bit
more verbose in that case to express the literal:
ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE="\"USB_STORAGE\""


The main reason against that, is, that in the expansion of
EXPORT_SYMBOL_NS, we define the ksymtab entry, which name is
constructed partly by the name of the namespace:

static const struct kernel_symbol __ksymtab_##sym##__##ns ...
^^^^

For that we depend on a cpp symbol to construct the name. I am not sure
there is a reasonable way of getting rid of that without ending up
constructing the ksymtab entries completely in asm as it is already done
in case of PREL32_RELOCATIONS. But I am happy to be corrected.

For reference that is done in patch 03/11 of this series:
https://lore.kernel.org/lkml/[email protected]/

Cheers,
Matthias