2018-11-20 15:33:14

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH] kbuild: announce removal of SUBDIRS if used

SUBDIRS has been kept as a backward compatibility since
commit ("[PATCH] kbuild: external module support") in 2002.

We do not need multiple ways to do the same thing, so I will remove
SUBDIRS after the Linux 5.3 release. I cleaned up in-tree code, and
updated the document so that nobody would try to use it.

Meanwhile, display the following warning if SUBDIRS is used.

Makefile:189: ================= WARNING ================
Makefile:190: 'SUBDIRS' will be removed after Linux 5.3
Makefile:191: Please use 'M=' or 'KBUILD_EXTMOD' instead
Makefile:192: ==========================================

Signed-off-by: Masahiro Yamada <[email protected]>
---

Documentation/kbuild/kbuild.txt | 7 +------
Makefile | 4 ++++
drivers/mtd/maps/scx200_docflash.c | 7 -------
drivers/watchdog/scx200_wdt.c | 7 -------
samples/connector/Makefile | 2 +-
5 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/Documentation/kbuild/kbuild.txt b/Documentation/kbuild/kbuild.txt
index 8390c36..c9e3d93 100644
--- a/Documentation/kbuild/kbuild.txt
+++ b/Documentation/kbuild/kbuild.txt
@@ -81,12 +81,7 @@ KBUILD_EXTMOD
--------------------------------------------------
Set the directory to look for the kernel source when building external
modules.
-The directory can be specified in several ways:
-1) Use "M=..." on the command line
-2) Environment variable KBUILD_EXTMOD
-3) Environment variable SUBDIRS
-The possibilities are listed in the order they take precedence.
-Using "M=..." will always override the others.
+Setting "M=..." takes precedence over KBUILD_EXTMOD.

KBUILD_OUTPUT
--------------------------------------------------
diff --git a/Makefile b/Makefile
index 370d13b..57be5fb 100644
--- a/Makefile
+++ b/Makefile
@@ -186,6 +186,10 @@ endif
# Old syntax make ... SUBDIRS=$PWD is still supported
# Setting the environment variable KBUILD_EXTMOD take precedence
ifdef SUBDIRS
+ $(warning ================= WARNING ================)
+ $(warning 'SUBDIRS' will be removed after Linux 5.3)
+ $(warning Please use 'M=' or 'KBUILD_EXTMOD' instead)
+ $(warning ==========================================)
KBUILD_EXTMOD ?= $(SUBDIRS)
endif

diff --git a/drivers/mtd/maps/scx200_docflash.c b/drivers/mtd/maps/scx200_docflash.c
index f1c1f73..7f1a0e6 100644
--- a/drivers/mtd/maps/scx200_docflash.c
+++ b/drivers/mtd/maps/scx200_docflash.c
@@ -216,10 +216,3 @@ static void __exit cleanup_scx200_docflash(void)

module_init(init_scx200_docflash);
module_exit(cleanup_scx200_docflash);
-
-/*
- Local variables:
- compile-command: "make -k -C ../../.. SUBDIRS=drivers/mtd/maps modules"
- c-basic-offset: 8
- End:
-*/
diff --git a/drivers/watchdog/scx200_wdt.c b/drivers/watchdog/scx200_wdt.c
index 836377c..ec4063e 100644
--- a/drivers/watchdog/scx200_wdt.c
+++ b/drivers/watchdog/scx200_wdt.c
@@ -262,10 +262,3 @@ static void __exit scx200_wdt_cleanup(void)

module_init(scx200_wdt_init);
module_exit(scx200_wdt_cleanup);
-
-/*
- Local variables:
- compile-command: "make -k -C ../.. SUBDIRS=drivers/char modules"
- c-basic-offset: 8
- End:
-*/
diff --git a/samples/connector/Makefile b/samples/connector/Makefile
index fe3c854..6ad7162 100644
--- a/samples/connector/Makefile
+++ b/samples/connector/Makefile
@@ -14,4 +14,4 @@ HOSTCFLAGS_ucon.o += -I$(objtree)/usr/include
all: modules

modules clean:
- $(MAKE) -C ../.. SUBDIRS=$(CURDIR) $@
+ $(MAKE) -C ../.. M=$(CURDIR) $@
--
2.7.4



2018-11-20 15:33:20

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH] kbuild: announce removal of SUBDIRS if used

On Wed, 21 Nov 2018 00:04:18 +0900
Masahiro Yamada <[email protected]> wrote:

> SUBDIRS has been kept as a backward compatibility since
> commit ("[PATCH] kbuild: external module support") in 2002.
>
> We do not need multiple ways to do the same thing, so I will remove
> SUBDIRS after the Linux 5.3 release. I cleaned up in-tree code, and
> updated the document so that nobody would try to use it.
>
> Meanwhile, display the following warning if SUBDIRS is used.
>
> Makefile:189: ================= WARNING ================
> Makefile:190: 'SUBDIRS' will be removed after Linux 5.3
> Makefile:191: Please use 'M=' or 'KBUILD_EXTMOD' instead
> Makefile:192: ==========================================
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> Documentation/kbuild/kbuild.txt | 7 +------
> Makefile | 4 ++++
> drivers/mtd/maps/scx200_docflash.c | 7 -------

For the docflash driver

Acked-by: Boris Brezillon <[email protected]>

> drivers/watchdog/scx200_wdt.c | 7 -------
> samples/connector/Makefile | 2 +-
> 5 files changed, 6 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/kbuild/kbuild.txt b/Documentation/kbuild/kbuild.txt
> index 8390c36..c9e3d93 100644
> --- a/Documentation/kbuild/kbuild.txt
> +++ b/Documentation/kbuild/kbuild.txt
> @@ -81,12 +81,7 @@ KBUILD_EXTMOD
> --------------------------------------------------
> Set the directory to look for the kernel source when building external
> modules.
> -The directory can be specified in several ways:
> -1) Use "M=..." on the command line
> -2) Environment variable KBUILD_EXTMOD
> -3) Environment variable SUBDIRS
> -The possibilities are listed in the order they take precedence.
> -Using "M=..." will always override the others.
> +Setting "M=..." takes precedence over KBUILD_EXTMOD.
>
> KBUILD_OUTPUT
> --------------------------------------------------
> diff --git a/Makefile b/Makefile
> index 370d13b..57be5fb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -186,6 +186,10 @@ endif
> # Old syntax make ... SUBDIRS=$PWD is still supported
> # Setting the environment variable KBUILD_EXTMOD take precedence
> ifdef SUBDIRS
> + $(warning ================= WARNING ================)
> + $(warning 'SUBDIRS' will be removed after Linux 5.3)
> + $(warning Please use 'M=' or 'KBUILD_EXTMOD' instead)
> + $(warning ==========================================)
> KBUILD_EXTMOD ?= $(SUBDIRS)
> endif
>
> diff --git a/drivers/mtd/maps/scx200_docflash.c b/drivers/mtd/maps/scx200_docflash.c
> index f1c1f73..7f1a0e6 100644
> --- a/drivers/mtd/maps/scx200_docflash.c
> +++ b/drivers/mtd/maps/scx200_docflash.c
> @@ -216,10 +216,3 @@ static void __exit cleanup_scx200_docflash(void)
>
> module_init(init_scx200_docflash);
> module_exit(cleanup_scx200_docflash);
> -
> -/*
> - Local variables:
> - compile-command: "make -k -C ../../.. SUBDIRS=drivers/mtd/maps modules"
> - c-basic-offset: 8
> - End:
> -*/
> diff --git a/drivers/watchdog/scx200_wdt.c b/drivers/watchdog/scx200_wdt.c
> index 836377c..ec4063e 100644
> --- a/drivers/watchdog/scx200_wdt.c
> +++ b/drivers/watchdog/scx200_wdt.c
> @@ -262,10 +262,3 @@ static void __exit scx200_wdt_cleanup(void)
>
> module_init(scx200_wdt_init);
> module_exit(scx200_wdt_cleanup);
> -
> -/*
> - Local variables:
> - compile-command: "make -k -C ../.. SUBDIRS=drivers/char modules"
> - c-basic-offset: 8
> - End:
> -*/
> diff --git a/samples/connector/Makefile b/samples/connector/Makefile
> index fe3c854..6ad7162 100644
> --- a/samples/connector/Makefile
> +++ b/samples/connector/Makefile
> @@ -14,4 +14,4 @@ HOSTCFLAGS_ucon.o += -I$(objtree)/usr/include
> all: modules
>
> modules clean:
> - $(MAKE) -C ../.. SUBDIRS=$(CURDIR) $@
> + $(MAKE) -C ../.. M=$(CURDIR) $@


2018-11-20 19:30:05

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] kbuild: announce removal of SUBDIRS if used

On Wed, Nov 21, 2018 at 12:04:18AM +0900, Masahiro Yamada wrote:
> SUBDIRS has been kept as a backward compatibility since
> commit ("[PATCH] kbuild: external module support") in 2002.
>
> We do not need multiple ways to do the same thing, so I will remove
> SUBDIRS after the Linux 5.3 release. I cleaned up in-tree code, and
> updated the document so that nobody would try to use it.
>
> Meanwhile, display the following warning if SUBDIRS is used.
>
> Makefile:189: ================= WARNING ================
> Makefile:190: 'SUBDIRS' will be removed after Linux 5.3
> Makefile:191: Please use 'M=' or 'KBUILD_EXTMOD' instead
> Makefile:192: ==========================================
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> Documentation/kbuild/kbuild.txt | 7 +------
> Makefile | 4 ++++
> drivers/mtd/maps/scx200_docflash.c | 7 -------
> drivers/watchdog/scx200_wdt.c | 7 -------

For watchdog:

Acked-by: Guenter Roeck <[email protected]>

> samples/connector/Makefile | 2 +-
> 5 files changed, 6 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/kbuild/kbuild.txt b/Documentation/kbuild/kbuild.txt
> index 8390c36..c9e3d93 100644
> --- a/Documentation/kbuild/kbuild.txt
> +++ b/Documentation/kbuild/kbuild.txt
> @@ -81,12 +81,7 @@ KBUILD_EXTMOD
> --------------------------------------------------
> Set the directory to look for the kernel source when building external
> modules.
> -The directory can be specified in several ways:
> -1) Use "M=..." on the command line
> -2) Environment variable KBUILD_EXTMOD
> -3) Environment variable SUBDIRS
> -The possibilities are listed in the order they take precedence.
> -Using "M=..." will always override the others.
> +Setting "M=..." takes precedence over KBUILD_EXTMOD.
>
> KBUILD_OUTPUT
> --------------------------------------------------
> diff --git a/Makefile b/Makefile
> index 370d13b..57be5fb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -186,6 +186,10 @@ endif
> # Old syntax make ... SUBDIRS=$PWD is still supported
> # Setting the environment variable KBUILD_EXTMOD take precedence
> ifdef SUBDIRS
> + $(warning ================= WARNING ================)
> + $(warning 'SUBDIRS' will be removed after Linux 5.3)
> + $(warning Please use 'M=' or 'KBUILD_EXTMOD' instead)
> + $(warning ==========================================)
> KBUILD_EXTMOD ?= $(SUBDIRS)
> endif
>
> diff --git a/drivers/mtd/maps/scx200_docflash.c b/drivers/mtd/maps/scx200_docflash.c
> index f1c1f73..7f1a0e6 100644
> --- a/drivers/mtd/maps/scx200_docflash.c
> +++ b/drivers/mtd/maps/scx200_docflash.c
> @@ -216,10 +216,3 @@ static void __exit cleanup_scx200_docflash(void)
>
> module_init(init_scx200_docflash);
> module_exit(cleanup_scx200_docflash);
> -
> -/*
> - Local variables:
> - compile-command: "make -k -C ../../.. SUBDIRS=drivers/mtd/maps modules"
> - c-basic-offset: 8
> - End:
> -*/
> diff --git a/drivers/watchdog/scx200_wdt.c b/drivers/watchdog/scx200_wdt.c
> index 836377c..ec4063e 100644
> --- a/drivers/watchdog/scx200_wdt.c
> +++ b/drivers/watchdog/scx200_wdt.c
> @@ -262,10 +262,3 @@ static void __exit scx200_wdt_cleanup(void)
>
> module_init(scx200_wdt_init);
> module_exit(scx200_wdt_cleanup);
> -
> -/*
> - Local variables:
> - compile-command: "make -k -C ../.. SUBDIRS=drivers/char modules"
> - c-basic-offset: 8
> - End:
> -*/
> diff --git a/samples/connector/Makefile b/samples/connector/Makefile
> index fe3c854..6ad7162 100644
> --- a/samples/connector/Makefile
> +++ b/samples/connector/Makefile
> @@ -14,4 +14,4 @@ HOSTCFLAGS_ucon.o += -I$(objtree)/usr/include
> all: modules
>
> modules clean:
> - $(MAKE) -C ../.. SUBDIRS=$(CURDIR) $@
> + $(MAKE) -C ../.. M=$(CURDIR) $@
> --
> 2.7.4
>

2018-11-24 07:59:50

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kbuild: announce removal of SUBDIRS if used

On Wed, Nov 21, 2018 at 1:25 AM Guenter Roeck <[email protected]> wrote:
>
> On Wed, Nov 21, 2018 at 12:04:18AM +0900, Masahiro Yamada wrote:
> > SUBDIRS has been kept as a backward compatibility since
> > commit ("[PATCH] kbuild: external module support") in 2002.
> >
> > We do not need multiple ways to do the same thing, so I will remove
> > SUBDIRS after the Linux 5.3 release. I cleaned up in-tree code, and
> > updated the document so that nobody would try to use it.
> >
> > Meanwhile, display the following warning if SUBDIRS is used.
> >
> > Makefile:189: ================= WARNING ================
> > Makefile:190: 'SUBDIRS' will be removed after Linux 5.3
> > Makefile:191: Please use 'M=' or 'KBUILD_EXTMOD' instead
> > Makefile:192: ==========================================
> >
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > ---

Applied to linux-kbuild.



> > Documentation/kbuild/kbuild.txt | 7 +------
> > Makefile | 4 ++++
> > drivers/mtd/maps/scx200_docflash.c | 7 -------
> > drivers/watchdog/scx200_wdt.c | 7 -------
>
> For watchdog:
>
> Acked-by: Guenter Roeck <[email protected]>
>
> > samples/connector/Makefile | 2 +-
> > 5 files changed, 6 insertions(+), 21 deletions(-)
> >
> > diff --git a/Documentation/kbuild/kbuild.txt b/Documentation/kbuild/kbuild.txt
> > index 8390c36..c9e3d93 100644
> > --- a/Documentation/kbuild/kbuild.txt
> > +++ b/Documentation/kbuild/kbuild.txt
> > @@ -81,12 +81,7 @@ KBUILD_EXTMOD
> > --------------------------------------------------
> > Set the directory to look for the kernel source when building external
> > modules.
> > -The directory can be specified in several ways:
> > -1) Use "M=..." on the command line
> > -2) Environment variable KBUILD_EXTMOD
> > -3) Environment variable SUBDIRS
> > -The possibilities are listed in the order they take precedence.
> > -Using "M=..." will always override the others.
> > +Setting "M=..." takes precedence over KBUILD_EXTMOD.
> >
> > KBUILD_OUTPUT
> > --------------------------------------------------
> > diff --git a/Makefile b/Makefile
> > index 370d13b..57be5fb 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -186,6 +186,10 @@ endif
> > # Old syntax make ... SUBDIRS=$PWD is still supported
> > # Setting the environment variable KBUILD_EXTMOD take precedence
> > ifdef SUBDIRS
> > + $(warning ================= WARNING ================)
> > + $(warning 'SUBDIRS' will be removed after Linux 5.3)
> > + $(warning Please use 'M=' or 'KBUILD_EXTMOD' instead)
> > + $(warning ==========================================)
> > KBUILD_EXTMOD ?= $(SUBDIRS)
> > endif
> >
> > diff --git a/drivers/mtd/maps/scx200_docflash.c b/drivers/mtd/maps/scx200_docflash.c
> > index f1c1f73..7f1a0e6 100644
> > --- a/drivers/mtd/maps/scx200_docflash.c
> > +++ b/drivers/mtd/maps/scx200_docflash.c
> > @@ -216,10 +216,3 @@ static void __exit cleanup_scx200_docflash(void)
> >
> > module_init(init_scx200_docflash);
> > module_exit(cleanup_scx200_docflash);
> > -
> > -/*
> > - Local variables:
> > - compile-command: "make -k -C ../../.. SUBDIRS=drivers/mtd/maps modules"
> > - c-basic-offset: 8
> > - End:
> > -*/
> > diff --git a/drivers/watchdog/scx200_wdt.c b/drivers/watchdog/scx200_wdt.c
> > index 836377c..ec4063e 100644
> > --- a/drivers/watchdog/scx200_wdt.c
> > +++ b/drivers/watchdog/scx200_wdt.c
> > @@ -262,10 +262,3 @@ static void __exit scx200_wdt_cleanup(void)
> >
> > module_init(scx200_wdt_init);
> > module_exit(scx200_wdt_cleanup);
> > -
> > -/*
> > - Local variables:
> > - compile-command: "make -k -C ../.. SUBDIRS=drivers/char modules"
> > - c-basic-offset: 8
> > - End:
> > -*/
> > diff --git a/samples/connector/Makefile b/samples/connector/Makefile
> > index fe3c854..6ad7162 100644
> > --- a/samples/connector/Makefile
> > +++ b/samples/connector/Makefile
> > @@ -14,4 +14,4 @@ HOSTCFLAGS_ucon.o += -I$(objtree)/usr/include
> > all: modules
> >
> > modules clean:
> > - $(MAKE) -C ../.. SUBDIRS=$(CURDIR) $@
> > + $(MAKE) -C ../.. M=$(CURDIR) $@
> > --
> > 2.7.4
> >



--
Best Regards
Masahiro Yamada

2018-11-24 08:08:45

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] kbuild: announce removal of SUBDIRS if used

On Wed, 2018-11-21 at 00:04 +0900, Masahiro Yamada wrote:
> SUBDIRS has been kept as a backward compatibility since
> commit ("[PATCH] kbuild: external module support") in 2002.
>
> We do not need multiple ways to do the same thing, so I will remove
> SUBDIRS after the Linux 5.3 release. I cleaned up in-tree code, and
> updated the document so that nobody would try to use it.
>
> Meanwhile, display the following warning if SUBDIRS is used.
>
> Makefile:189: ================= WARNING ================
> Makefile:190: 'SUBDIRS' will be removed after Linux 5.3
> Makefile:191: Please use 'M=' or 'KBUILD_EXTMOD' instead
> Makefile:192: ==========================================
>
> Signed-off-by: Masahiro Yamada <[email protected]>

No, please don't do this. This is effectively a kernel←→user ABI.

The instructions for building a kernel module have been this, for
*ages*:

echo 'obj-m := mymod.o' > Makefile
make -C /lib/modules/`uname -r`/build SUBDIRS=`pwd`

People have muscle memory, they have it encoded into various of their
own makefiles and build scripts. Please don't make it stop working
unless there's actually a really good reason to do so.



Attachments:
smime.p7s (5.09 kB)

2018-11-24 08:26:16

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kbuild: announce removal of SUBDIRS if used

On Fri, Nov 23, 2018 at 5:04 PM David Woodhouse <[email protected]> wrote:
>
> On Wed, 2018-11-21 at 00:04 +0900, Masahiro Yamada wrote:
> > SUBDIRS has been kept as a backward compatibility since
> > commit ("[PATCH] kbuild: external module support") in 2002.
> >
> > We do not need multiple ways to do the same thing, so I will remove
> > SUBDIRS after the Linux 5.3 release. I cleaned up in-tree code, and
> > updated the document so that nobody would try to use it.
> >
> > Meanwhile, display the following warning if SUBDIRS is used.
> >
> > Makefile:189: ================= WARNING ================
> > Makefile:190: 'SUBDIRS' will be removed after Linux 5.3
> > Makefile:191: Please use 'M=' or 'KBUILD_EXTMOD' instead
> > Makefile:192: ==========================================
> >
> > Signed-off-by: Masahiro Yamada <[email protected]>
>
> No, please don't do this. This is effectively a kernel←→user ABI.


External modules are just downstream kernel code.

There is no guarantee for eternal backward compatibility.


Actually, kernel headers and APIs are always changing.

We are not allowed to break the upstream code during API migration,
but we do not care about external modules.


If you want to compile external modules
with a new version of the kernel,
you need to adjust various parts of the code anyway.
Is it a problem to ask one-liner fix in a build script?


> The instructions for building a kernel module have been this, for
> *ages*:
>
> echo 'obj-m := mymod.o' > Makefile
> make -C /lib/modules/`uname -r`/build SUBDIRS=`pwd`


It has been *ages* since the external module build
was improved in the current way.

Since then, M=<...> is a preferred way over SUBDIRS.

This case is even nicer.
I set a one-year grace period with a clear warning message.


> People have muscle memory, they have it encoded into various of their
> own makefiles and build scripts. Please don't make it stop working
> unless there's actually a really good reason to do so.

Dumping old code is a good reason.


--
Best Regards
Masahiro Yamada