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

[Mike Anderson]

Andrew Morton <akpm at linux-foundation.org> wrote:
> 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?
> 

Yes.

> > ...
> >
> > +#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?
> 

ok, I will switch to the ifdef CONFIG_DM_NETLINK in the tables.


> >  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?  

No. The support is a derivative of the netlink support in
scsi_transport_iscsi.c.

-andmike
--
Michael Anderson
andmike at us.ibm.com