[PATCH net-next 0/6] make non-modular code explicitly non-modular

[PATCH net-next 0/6] make non-modular code explicitly non-modular

[Paul Gortmaker]

In a previous merge window, we made changes to allow better
delineation between modular and non-modular code in commit
0fd972a7d91d6e15393c449492a04d94c0b89351 ("module: relocate module_init
from init.h to module.h").  This allows us to now ensure module code
looks modular and non-modular code does not accidentally look modular
just to avoid suffering build breakage.

Here we target code that is, by nature of their Makefile and/or
Kconfig settings, only available to be built-in, but implicitly
presenting itself as being possibly modular by way of using modular
headers, macros, and functions.

The goal here is to remove that illusion of modularity from these
files, but in a way that leaves the actual runtime unchanged.
In doing so, we remove code that has never been tested and adds
no value to the tree.  And we continue the process of expecting a
level of consistency between the Kconfig/Makefile of code and the
code in use itself.

Fortuntately the net subsystem has relatively few instances, given
the overall amount of code and drivers it contains.  For comparison
there are over 300 instances tree wide, resulting in a possible net
removal of on the order of 5000 lines of unused code.

Build tested on net-next 34c2d9fb0498 on m68k, since that is the arch
where the three ethernet drivers changed here are available.

Paul.
--

Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Anish Bhatt <anish@chelsio.com>
Cc: Craig Gallek <kraig@google.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: John Fastabend <john.r.fastabend@intel.com>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Cc: Shani Michaeli <shanim@mellanox.com>
Cc: linux-m68k@lists.linux-m68k.org
Cc: netdev@vger.kernel.org

Paul Gortmaker (6):
  net/core: make sock_diag.c explicitly non-modular
  net/dcb: make dcbnl.c explicitly non-modular
  net/sched: make sch_blackhole.c explicitly non-modular
  net/ethernet: make amd/hplance.c driver explicitly non-modular
  net/ethernet: make 8390/mac8390.c driver explicitly non-modular
  net/ethernet: make apple/macmace.c driver explicitly non-modular

 drivers/net/ethernet/8390/mac8390.c  | 40 ++----------------------------------
 drivers/net/ethernet/amd/hplance.c   | 13 ++----------
 drivers/net/ethernet/apple/macmace.c | 38 +++-------------------------------
 net/core/sock_diag.c                 | 14 +++----------
 net/dcb/dcbnl.c                      | 30 +++------------------------
 net/sched/sch_blackhole.c            | 15 +++-----------
 6 files changed, 16 insertions(+), 134 deletions(-)

-- 
2.6.0.rc3

[PATCH 1/6] net/core: make sock_diag.c explicitly non-modular

[Paul Gortmaker]

The Makefile currently controlling compilation of this code lists
it under "obj-y" ...meaning that it currently is not being built as
a module by anyone.

Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.  We can
change to one of the other priority initcalls (subsys?) at any later
date, if desired.

We can't remove module.h since the file uses other module related
stuff even though it is not modular itself.

We move the information from the MODULE_LICENSE tag to the top of the
file, since that information is not captured anywhere else.  The
MODULE_ALIAS_NET_PF_PROTO becomes a no-op in the non modular case, so
it is removed.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Craig Gallek <kraig@google.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/core/sock_diag.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/net/core/sock_diag.c b/net/core/sock_diag.c
index 817622f3dbb7..0c1d58d43f67 100644
--- a/net/core/sock_diag.c
+++ b/net/core/sock_diag.c
@@ -1,3 +1,5 @@
+/* License: GPL */
+
 #include <linux/mutex.h>
 #include <linux/socket.h>
 #include <linux/skbuff.h>
@@ -323,14 +325,4 @@ static int __init sock_diag_init(void)
 	BUG_ON(!broadcast_wq);
 	return register_pernet_subsys(&diag_net_ops);
 }
-
-static void __exit sock_diag_exit(void)
-{
-	unregister_pernet_subsys(&diag_net_ops);
-	destroy_workqueue(broadcast_wq);
-}
-
-module_init(sock_diag_init);
-module_exit(sock_diag_exit);
-MODULE_LICENSE("GPL");
-MODULE_ALIAS_NET_PF_PROTO(PF_NETLINK, NETLINK_SOCK_DIAG);
+device_initcall(sock_diag_init);
-- 
2.6.0.rc3

[PATCH 2/6] net/dcb: make dcbnl.c explicitly non-modular

[Paul Gortmaker]

The Kconfig currently controlling compilation of this code is:

net/dcb/Kconfig:config DCB
net/dcb/Kconfig:        bool "Data Center Bridging support"

...meaning that it currently is not being built as a module by anyone.

Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.  We can
change to one of the other priority initcalls (subsys?) at any later
date, if desired.

We also delete the MODULE_LICENSE tag etc. since all that information
is (or is now) already contained at the top of the file in the comments.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Cc: Anish Bhatt <anish@chelsio.com>
Cc: John Fastabend <john.r.fastabend@intel.com>
Cc: Shani Michaeli <shanim@mellanox.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/dcb/dcbnl.c | 30 +++---------------------------
 1 file changed, 3 insertions(+), 27 deletions(-)

diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
index 5b21f6f88e97..4f6c1862dfd2 100644
--- a/net/dcb/dcbnl.c
+++ b/net/dcb/dcbnl.c
@@ -13,6 +13,7 @@
  * You should have received a copy of the GNU General Public License along with
  * this program; if not, see <http://www.gnu.org/licenses/>.
  *
+ * Description: Data Center Bridging netlink interface
  * Author: Lucy Liu <lucy.liu@intel.com>
  */
 
@@ -24,7 +25,7 @@
 #include <linux/dcbnl.h>
 #include <net/dcbevent.h>
 #include <linux/rtnetlink.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <net/sock.h>
 
 /* Data Center Bridging (DCB) is a collection of Ethernet enhancements
@@ -48,10 +49,6 @@
  * features for capable devices.
  */
 
-MODULE_AUTHOR("Lucy Liu, <lucy.liu@intel.com>");
-MODULE_DESCRIPTION("Data Center Bridging netlink interface");
-MODULE_LICENSE("GPL");
-
 /**************** DCB attribute policies *************************************/
 
 /* DCB netlink attributes policy */
@@ -1935,19 +1932,6 @@ int dcb_ieee_delapp(struct net_device *dev, struct dcb_app *del)
 }
 EXPORT_SYMBOL(dcb_ieee_delapp);
 
-static void dcb_flushapp(void)
-{
-	struct dcb_app_type *app;
-	struct dcb_app_type *tmp;
-
-	spin_lock_bh(&dcb_lock);
-	list_for_each_entry_safe(app, tmp, &dcb_app_list, list) {
-		list_del(&app->list);
-		kfree(app);
-	}
-	spin_unlock_bh(&dcb_lock);
-}
-
 static int __init dcbnl_init(void)
 {
 	INIT_LIST_HEAD(&dcb_app_list);
@@ -1957,12 +1941,4 @@ static int __init dcbnl_init(void)
 
 	return 0;
 }
-module_init(dcbnl_init);
-
-static void __exit dcbnl_exit(void)
-{
-	rtnl_unregister(PF_UNSPEC, RTM_GETDCB);
-	rtnl_unregister(PF_UNSPEC, RTM_SETDCB);
-	dcb_flushapp();
-}
-module_exit(dcbnl_exit);
+device_initcall(dcbnl_init);
-- 
2.6.0.rc3

[PATCH 3/6] net/sched: make sch_blackhole.c explicitly non-modular

[Paul Gortmaker]

The Kconfig currently controlling compilation of this code is:

net/sched/Kconfig:menuconfig NET_SCHED
net/sched/Kconfig:      bool "QoS and/or fair queueing"

...meaning that it currently is not being built as a module by anyone.

Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.  We can
change to one of the other priority initcalls (subsys?) at any later
date, if desired.

We also delete the MODULE_LICENSE tag since all that information
is already contained at the top of the file in the comments.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/sched/sch_blackhole.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/net/sched/sch_blackhole.c b/net/sched/sch_blackhole.c
index 094a874b48bc..3fee70d9814f 100644
--- a/net/sched/sch_blackhole.c
+++ b/net/sched/sch_blackhole.c
@@ -11,7 +11,7 @@
  * Note: Quantum tunneling is not supported.
  */
 
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/types.h>
 #include <linux/kernel.h>
 #include <linux/skbuff.h>
@@ -37,17 +37,8 @@ static struct Qdisc_ops blackhole_qdisc_ops __read_mostly = {
 	.owner		= THIS_MODULE,
 };
 
-static int __init blackhole_module_init(void)
+static int __init blackhole_init(void)
 {
 	return register_qdisc(&blackhole_qdisc_ops);
 }
-
-static void __exit blackhole_module_exit(void)
-{
-	unregister_qdisc(&blackhole_qdisc_ops);
-}
-
-module_init(blackhole_module_init)
-module_exit(blackhole_module_exit)
-
-MODULE_LICENSE("GPL");
+device_initcall(blackhole_init)
-- 
2.6.0.rc3

[PATCH 4/6] net/ethernet: make amd/hplance.c driver explicitly non-modular

[Paul Gortmaker]

The Kconfig for this driver is currently:

config HPLANCE
        bool "HP on-board LANCE support"

...meaning that it currently is not being built as a module by
anyone.  Lets remove the modular and unused code here, so that
when reading the driver there is no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.

We don't swap module.h for init.h since this file has init.h already.

We move the information from the MODULE_LICENSE tag to the top of the
file in this case, since it is not duplicating similar information
already there.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/net/ethernet/amd/hplance.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/amd/hplance.c b/drivers/net/ethernet/amd/hplance.c
index 6c9de117ffc6..fa9397508a79 100644
--- a/drivers/net/ethernet/amd/hplance.c
+++ b/drivers/net/ethernet/amd/hplance.c
@@ -3,9 +3,9 @@
  * Copyright (C) 05/1998 Peter Maydell <pmaydell@chiark.greenend.org.uk>
  * Based on the Sun Lance driver and the NetBSD HP Lance driver
  * Uses the generic 7990.c LANCE code.
+ * License: GPL
  */
 
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/interrupt.h>
@@ -220,13 +220,4 @@ static int __init hplance_init_module(void)
 {
 	return dio_register_driver(&hplance_driver);
 }
-
-static void __exit hplance_cleanup_module(void)
-{
-	dio_unregister_driver(&hplance_driver);
-}
-
-module_init(hplance_init_module);
-module_exit(hplance_cleanup_module);
-
-MODULE_LICENSE("GPL");
+device_initcall(hplance_init_module);
-- 
2.6.0.rc3

[PATCH 5/6] net/ethernet: make 8390/mac8390.c driver explicitly non-modular

[Paul Gortmaker]

The Kconfig for this option is currently:

config MAC8390
    bool "Macintosh NS 8390 based ethernet cards"

...meaning that it currently is not being built as a module by anyone.
Lets remove the modular code that is essentially orphaned -- it was never
even updated to use module_init/module_exit.  After this, when reading
the driver there should be no doubt it is builtin-only.

We don't swap module.h for init.h since this file has init.h already.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.

We move the information from MODULE_AUTHOR to the comments at the top
of the file for documentation purposes.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/net/ethernet/8390/mac8390.c | 40 ++-----------------------------------
 1 file changed, 2 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/8390/mac8390.c b/drivers/net/ethernet/8390/mac8390.c
index 65cf60f6718c..a971bf629715 100644
--- a/drivers/net/ethernet/8390/mac8390.c
+++ b/drivers/net/ethernet/8390/mac8390.c
@@ -4,6 +4,8 @@
    taken from or inspired by skeleton.c by Donald Becker, acenic.c by
    Jes Sorensen, and ne2k-pci.c by Donald Becker and Paul Gortmaker.
 
+   Author: David Huggins-Daines <dhd@debian.org> and others.
+
    This software may be used and distributed according to the terms of
    the GNU Public License, incorporated herein by reference.  */
 
@@ -19,7 +21,6 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/fcntl.h>
@@ -449,43 +450,6 @@ out:
 	return ERR_PTR(err);
 }
 
-#ifdef MODULE
-MODULE_AUTHOR("David Huggins-Daines <dhd@debian.org> and others");
-MODULE_DESCRIPTION("Macintosh NS8390-based Nubus Ethernet driver");
-MODULE_LICENSE("GPL");
-
-/* overkill, of course */
-static struct net_device *dev_mac8390[15];
-int init_module(void)
-{
-	int i;
-	for (i = 0; i < 15; i++) {
-		struct net_device *dev = mac8390_probe(-1);
-		if (IS_ERR(dev))
-			break;
-		dev_mac890[i] = dev;
-	}
-	if (!i) {
-		pr_notice("No useable cards found, driver NOT installed.\n");
-		return -ENODEV;
-	}
-	return 0;
-}
-
-void cleanup_module(void)
-{
-	int i;
-	for (i = 0; i < 15; i++) {
-		struct net_device *dev = dev_mac890[i];
-		if (dev) {
-			unregister_netdev(dev);
-			free_netdev(dev);
-		}
-	}
-}
-
-#endif /* MODULE */
-
 static const struct net_device_ops mac8390_netdev_ops = {
 	.ndo_open 		= mac8390_open,
 	.ndo_stop		= mac8390_close,
-- 
2.6.0.rc3

[PATCH 6/6] net/ethernet: make apple/macmace.c driver explicitly non-modular

[Paul Gortmaker]

The Kconfig for this option is currently:

config MACMACE
        bool "Macintosh (AV) onboard MACE ethernet"

...meaning that it currently is not being built as a module by anyone.
Lets remove the modular code that is essentially orphaned, so when
reading the driver there should be no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.

We also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the file in the comments.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/net/ethernet/apple/macmace.c | 38 +++---------------------------------
 1 file changed, 3 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ethernet/apple/macmace.c b/drivers/net/ethernet/apple/macmace.c
index 89914ca17a49..1f78bc1d04ce 100644
--- a/drivers/net/ethernet/apple/macmace.c
+++ b/drivers/net/ethernet/apple/macmace.c
@@ -21,7 +21,7 @@
 
 
 #include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/delay.h>
@@ -739,50 +739,18 @@ static irqreturn_t mace_dma_intr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-MODULE_LICENSE("GPL");
-MODULE_DESCRIPTION("Macintosh MACE ethernet driver");
-MODULE_ALIAS("platform:macmace");
-
-static int mac_mace_device_remove(struct platform_device *pdev)
-{
-	struct net_device *dev = platform_get_drvdata(pdev);
-	struct mace_data *mp = netdev_priv(dev);
-
-	unregister_netdev(dev);
-
-	free_irq(dev->irq, dev);
-	free_irq(IRQ_MAC_MACE_DMA, dev);
-
-	dma_free_coherent(mp->device, N_RX_RING * MACE_BUFF_SIZE,
-	                  mp->rx_ring, mp->rx_ring_phys);
-	dma_free_coherent(mp->device, N_TX_RING * MACE_BUFF_SIZE,
-	                  mp->tx_ring, mp->tx_ring_phys);
-
-	free_netdev(dev);
-
-	return 0;
-}
-
 static struct platform_driver mac_mace_driver = {
 	.probe  = mace_probe,
-	.remove = mac_mace_device_remove,
 	.driver	= {
 		.name	= mac_mace_string,
 	},
 };
 
-static int __init mac_mace_init_module(void)
+static int __init mac_mace_init(void)
 {
 	if (!MACH_IS_MAC)
 		return -ENODEV;
 
 	return platform_driver_register(&mac_mace_driver);
 }
-
-static void __exit mac_mace_cleanup_module(void)
-{
-	platform_driver_unregister(&mac_mace_driver);
-}
-
-module_init(mac_mace_init_module);
-module_exit(mac_mace_cleanup_module);
+device_initcall(mac_mace_init);
-- 
2.6.0.rc3

Re: [PATCH net-next 0/6] make non-modular code explicitly non-modular

[Geert Uytterhoeven]

Hi Paul,

On Mon, Sep 28, 2015 at 9:51 PM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:
> In a previous merge window, we made changes to allow better
> delineation between modular and non-modular code in commit
> 0fd972a7d91d6e15393c449492a04d94c0b89351 ("module: relocate module_init
> from init.h to module.h").  This allows us to now ensure module code
> looks modular and non-modular code does not accidentally look modular
> just to avoid suffering build breakage.
>
> Here we target code that is, by nature of their Makefile and/or
> Kconfig settings, only available to be built-in, but implicitly
> presenting itself as being possibly modular by way of using modular
> headers, macros, and functions.
>
> The goal here is to remove that illusion of modularity from these
> files, but in a way that leaves the actual runtime unchanged.
> In doing so, we remove code that has never been tested and adds
> no value to the tree.  And we continue the process of expecting a
> level of consistency between the Kconfig/Makefile of code and the
> code in use itself.
>
> Fortuntately the net subsystem has relatively few instances, given
> the overall amount of code and drivers it contains.  For comparison
> there are over 300 instances tree wide, resulting in a possible net
> removal of on the order of 5000 lines of unused code.
>
> Build tested on net-next 34c2d9fb0498 on m68k, since that is the arch
> where the three ethernet drivers changed here are available.

>   net/ethernet: make amd/hplance.c driver explicitly non-modular
>   net/ethernet: make 8390/mac8390.c driver explicitly non-modular
>   net/ethernet: make apple/macmace.c driver explicitly non-modular

Why did you choose this approach?
What about changing the "bool"s to "tristate"s in Kconfig instead?

I gave it a try, and with some small changes the three m68k ethernet drivers
build fine as modular drivers. I can send patches if you like it.

Thanks!

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

Re: [PATCH net-next 0/6] make non-modular code explicitly non-modular

[Paul Gortmaker]

[Re: [PATCH net-next 0/6] make non-modular code explicitly non-modular] On 28/09/2015 (Mon 23:09) Geert Uytterhoeven wrote:

> Hi Paul,
> 
> On Mon, Sep 28, 2015 at 9:51 PM, Paul Gortmaker
> <paul.gortmaker@windriver.com> wrote:
> > In a previous merge window, we made changes to allow better
> > delineation between modular and non-modular code in commit
> > 0fd972a7d91d6e15393c449492a04d94c0b89351 ("module: relocate module_init
> > from init.h to module.h").  This allows us to now ensure module code
> > looks modular and non-modular code does not accidentally look modular
> > just to avoid suffering build breakage.
> >
> > Here we target code that is, by nature of their Makefile and/or
> > Kconfig settings, only available to be built-in, but implicitly
> > presenting itself as being possibly modular by way of using modular
> > headers, macros, and functions.
> >
> > The goal here is to remove that illusion of modularity from these
> > files, but in a way that leaves the actual runtime unchanged.
> > In doing so, we remove code that has never been tested and adds
> > no value to the tree.  And we continue the process of expecting a
> > level of consistency between the Kconfig/Makefile of code and the
> > code in use itself.
> >
> > Fortuntately the net subsystem has relatively few instances, given
> > the overall amount of code and drivers it contains.  For comparison
> > there are over 300 instances tree wide, resulting in a possible net
> > removal of on the order of 5000 lines of unused code.
> >
> > Build tested on net-next 34c2d9fb0498 on m68k, since that is the arch
> > where the three ethernet drivers changed here are available.
> 
> >   net/ethernet: make amd/hplance.c driver explicitly non-modular
> >   net/ethernet: make 8390/mac8390.c driver explicitly non-modular
> >   net/ethernet: make apple/macmace.c driver explicitly non-modular
> 
> Why did you choose this approach?
> What about changing the "bool"s to "tristate"s in Kconfig instead?

Long answer is here:

https://lkml.org/lkml/2015/8/24/888

To summarize, it adds functionality to code I can't test, and with 300
or so of these, it already has been a large time sink.  Add to that
extending the functionality and testing the new functionality, and it
does not scale.   Plus if something hasn't allowed tristate for over
10 years, where is the value in adding it now?

> I gave it a try, and with some small changes the three m68k ethernet drivers
> build fine as modular drivers. I can send patches if you like it.

Per above, I don't see the value in it, but if you want to do it and
test it and own submitting the patches, then I can drop the corresponding
ones from my queue.  Either way we get the code matching the Kconfig
which is what I'm after out of this.

Note that if you do decide to do this, the one driver really needs more
than just tristate one line change, it had super ancient init code that
predates module_init and probably needs an update.

Thanks,
Paul.
--

> 
> Thanks!
> 
> 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

Re: [PATCH net-next 0/6] make non-modular code explicitly non-modular

[Finn Thain]

Hi Paul,

On Mon, 28 Sep 2015, Paul Gortmaker wrote:

> On 28/09/2015 (Mon 23:09) Geert Uytterhoeven wrote:
> 
> > Hi Paul,
...
> > 
> > Why did you choose this approach?
> > What about changing the "bool"s to "tristate"s in Kconfig instead?
> 
> Long answer is here:
> 
> https://lkml.org/lkml/2015/8/24/888

You wrote, "If there was demand for them to be tristate, then it would 
have happened by now." I don't follow your reasoning. You might just as 
well remove entire drivers and then argue, "If there was demand for 
drivers without bugs, then someone would have written them by now".

Perhaps you meant, "If there was sufficient demand for them to be 
tristate, then sufficient resources would have been marshalled, as 
required to get an enhancement written, tested, submitted, reviewed and 
merged in the mainline kernel."

> 
> To summarize, it adds functionality to code I can't test, and with 300 
> or so of these, it already has been a large time sink.  Add to that 
> extending the functionality and testing the new functionality, and it 
> does not scale. Plus if something hasn't allowed tristate for over 10 
> years, where is the value in adding it now?

There is value to be gained by completing the tristate support, and there 
is value destroyed by removing the partial tristate support.

I'm not involved in building distro kernels, but I know that Debian's 
would benefit from these tristates, because it would reduce the size of 
the m68k multi-platform kernel binary.

And even if it is dead code you aim to remove, a lot of people have worked 
on it (according to git blame), including myself. We should not disregard 
that effort when we could leverage it instead.

For the macmace driver in particular, I did the platform driver 
conversion, and it should work as a module. I did not change it to 
tristate at the time because I did not want to deal with the question of 
the 'psc' global, which lacks an EXPORT_SYMBOL(psc). Anyway, I'll send a 
patch if Geert doesn't do so first.

> 
> > I gave it a try, and with some small changes the three m68k ethernet 
> > drivers build fine as modular drivers. I can send patches if you like 
> > it.
> 
> Per above, I don't see the value in it, but if you want to do it and 
> test it and own submitting the patches, then I can drop the 
> corresponding ones from my queue.

I can't test right now but I have the hardware and will attend to any 
issues if need be. I do not expect any issues, because the modular option 
seems to involve the same code paths in the driver.

If the CONFIG_MACMACE=m option was implemented badly and did not work 
correctly, at least it couldn't be called a regression, presuming that 'm' 
builds okay, and that the default was 'y' or 'n'.

> Either way we get the code matching the Kconfig which is what I'm after 
> out of this.

Yes, me too.

> 
> Note that if you do decide to do this, the one driver really needs more 
> than just tristate one line change, it had super ancient init code that 
> predates module_init and probably needs an update.

I think the solution for mac8390 is to do in the modular case exactly what 
Space.c does in the built-in case. That would mean that the modular driver 
would support only one card, just like the built-in driver. (That 
limitation is a problem which affects all Nubus card drivers, because they 
have to do all their own bus matching, because Nubus still lacks the 
necessary driver model support.)

I haven't looked at amd/hplance, but I expect that the issues are similar.

Geert, do plan to send patches for any of these drivers?

Regards,
Finn

> 
> Thanks,
> Paul.
> --
> 
> > 
> > Thanks!
> > 
> > Gr{oetje,eeting}s,
> > 
> >                         Geert

Re: [PATCH net-next 0/6] make non-modular code explicitly non-modular

[Paul Gortmaker]

[Re: [PATCH net-next 0/6] make non-modular code explicitly non-modular] On 29/09/2015 (Tue 16:32) Finn Thain wrote:

> 
> Hi Paul,
> 
> On Mon, 28 Sep 2015, Paul Gortmaker wrote:
> 
> > On 28/09/2015 (Mon 23:09) Geert Uytterhoeven wrote:
> > 
> > > Hi Paul,
> ...
> > > 
> > > Why did you choose this approach?
> > > What about changing the "bool"s to "tristate"s in Kconfig instead?
> > 
> > Long answer is here:
> > 
> > https://lkml.org/lkml/2015/8/24/888
> 
> You wrote, "If there was demand for them to be tristate, then it would 
> have happened by now." I don't follow your reasoning. You might just as 
> well remove entire drivers and then argue, "If there was demand for 
> drivers without bugs, then someone would have written them by now".

I don't see those two sentences being alike, but in the end it does
not matter, since Geert has decided to do the conversion and test it.

And whatever code gets removed is never truly gone anyway; it lives on
in the git history forever.

Thanks,
Paul.
--

> 
> Perhaps you meant, "If there was sufficient demand for them to be 
> tristate, then sufficient resources would have been marshalled, as 
> required to get an enhancement written, tested, submitted, reviewed and 
> merged in the mainline kernel."
> 
> > 
> > To summarize, it adds functionality to code I can't test, and with 300 
> > or so of these, it already has been a large time sink.  Add to that 
> > extending the functionality and testing the new functionality, and it 
> > does not scale. Plus if something hasn't allowed tristate for over 10 
> > years, where is the value in adding it now?
> 
> There is value to be gained by completing the tristate support, and there 
> is value destroyed by removing the partial tristate support.
> 
> I'm not involved in building distro kernels, but I know that Debian's 
> would benefit from these tristates, because it would reduce the size of 
> the m68k multi-platform kernel binary.
> 
> And even if it is dead code you aim to remove, a lot of people have worked 
> on it (according to git blame), including myself. We should not disregard 
> that effort when we could leverage it instead.
> 
> For the macmace driver in particular, I did the platform driver 
> conversion, and it should work as a module. I did not change it to 
> tristate at the time because I did not want to deal with the question of 
> the 'psc' global, which lacks an EXPORT_SYMBOL(psc). Anyway, I'll send a 
> patch if Geert doesn't do so first.
> 
> > 
> > > I gave it a try, and with some small changes the three m68k ethernet 
> > > drivers build fine as modular drivers. I can send patches if you like 
> > > it.
> > 
> > Per above, I don't see the value in it, but if you want to do it and 
> > test it and own submitting the patches, then I can drop the 
> > corresponding ones from my queue.
> 
> I can't test right now but I have the hardware and will attend to any 
> issues if need be. I do not expect any issues, because the modular option 
> seems to involve the same code paths in the driver.
> 
> If the CONFIG_MACMACE=m option was implemented badly and did not work 
> correctly, at least it couldn't be called a regression, presuming that 'm' 
> builds okay, and that the default was 'y' or 'n'.
> 
> > Either way we get the code matching the Kconfig which is what I'm after 
> > out of this.
> 
> Yes, me too.
> 
> > 
> > Note that if you do decide to do this, the one driver really needs more 
> > than just tristate one line change, it had super ancient init code that 
> > predates module_init and probably needs an update.
> 
> I think the solution for mac8390 is to do in the modular case exactly what 
> Space.c does in the built-in case. That would mean that the modular driver 
> would support only one card, just like the built-in driver. (That 
> limitation is a problem which affects all Nubus card drivers, because they 
> have to do all their own bus matching, because Nubus still lacks the 
> necessary driver model support.)
> 
> I haven't looked at amd/hplance, but I expect that the issues are similar.
> 
> Geert, do plan to send patches for any of these drivers?
> 
> Regards,
> Finn
> 
> > 
> > Thanks,
> > Paul.
> > --
> > 
> > > 
> > > Thanks!
> > > 
> > > Gr{oetje,eeting}s,
> > > 
> > >                         Geert
>