[dm-devel] Re: [2.6.23 PATCH 13/18] dm: netlink

[Andrew Morton]

On Wed, 11 Jul 2007 22:01:37 +0100
Alasdair G Kergon <agk at redhat.com> wrote:

> From: Mike Anderson <andmike at us.ibm.com>
> 
> This patch adds a dm-netlink skeleton support to the Makefile, and the dm
> directory.
> 
> ...
>  
> +config DM_NETLINK
> +	bool "DM netlink events (EXPERIMENTAL)"
> +	depends on BLK_DEV_DM && EXPERIMENTAL
> +	---help---
> +	Generate netlink events for DM events.

Need a dependency on NET there?

> ...
>
> +#ifdef CONFIG_DM_NETLINK
> +
> +int dm_netlink_init(void);
> +void dm_netlink_exit(void);
> +
> +#else	/* CONFIG_DM_NETLINK */
> +
> +static inline int __init dm_netlink_init(void)

The __init here isn't needed (doesn't make sense, is missing the required
#include anyway)

> +{
> +	return 0;
> +}
> +static inline void dm_netlink_exit(void)
> +{
> +}
> +
> +#endif	/* CONFIG_DM_NETLINK */
> +
> +#endif	/* DM_NETLINK_H */
> Index: linux/drivers/md/dm.c
> ===================================================================
> --- linux.orig/drivers/md/dm.c	2007-07-11 21:37:47.000000000 +0100
> +++ linux/drivers/md/dm.c	2007-07-11 21:37:50.000000000 +0100
> @@ -7,6 +7,7 @@
>  
>  #include "dm.h"
>  #include "dm-bio-list.h"
> +#include "dm-netlink.h"
>  
>  #include <linux/init.h>
>  #include <linux/module.h>
> @@ -176,6 +177,7 @@ int (*_inits[])(void) __initdata = {
>  	dm_linear_init,
>  	dm_stripe_init,
>  	dm_interface_init,
> +	dm_netlink_init,
>  };
>  
>  void (*_exits[])(void) = {
> @@ -184,6 +186,7 @@ void (*_exits[])(void) = {
>  	dm_linear_exit,
>  	dm_stripe_exit,
>  	dm_interface_exit,
> +	dm_netlink_exit,
>  };

hm, so if CONFIG_DM_NETLINK=n we end up taking the address of an inlined
function.  So the __init above _did_ make sense, in a peculiar way.  I
don't know that gcc will actually put that converted-to-non-inline function
into the desired section though.

There's no way in which those inlined functions will ever get inlined.
Perhaps all this would be better if there was no implementation of
dm_netlink_init() and dm_netlink_exit() if CONFIG_DM_NETLINK=n and you just
whack the requisite ifdefs into these tables here?

>  static int __init dm_init(void)
> Index: linux/include/linux/netlink.h
> ===================================================================
> --- linux.orig/include/linux/netlink.h	2007-07-11 21:37:31.000000000 +0100
> +++ linux/include/linux/netlink.h	2007-07-11 21:37:50.000000000 +0100
> @@ -21,7 +21,7 @@
>  #define NETLINK_DNRTMSG		14	/* DECnet routing messages */
>  #define NETLINK_KOBJECT_UEVENT	15	/* Kernel messages to userspace */
>  #define NETLINK_GENERIC		16
> -/* leave room for NETLINK_DM (DM Events) */
> +#define NETLINK_DM		17	/* Device Mapper */
>  #define NETLINK_SCSITRANSPORT	18	/* SCSI Transports */
>  #define NETLINK_ECRYPTFS	19

Have the net guys checked this?