Re: [PATCH] genetlink: don't touch module ref count
On Fri, 13 Jan 2006, David S. Miller wrote: > From: Per Liden <[EMAIL PROTECTED]> > Date: Fri, 13 Jan 2006 21:56:53 +0100 (CET) > > > Signed-off-by: Per Liden <[EMAIL PROTECTED]> > > BTW, don't do this, it should be a perfectly functioning > email address so that people can contact you. Ok, sorry, I'll avoid that in the future. /Per - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] genetlink: don't touch module ref count
From: Per Liden <[EMAIL PROTECTED]> Date: Fri, 13 Jan 2006 21:56:53 +0100 (CET) > On Fri, 13 Jan 2006, David S. Miller wrote: > > > From: Jamal Hadi Salim <[EMAIL PROTECTED]> > > Date: Fri, 13 Jan 2006 09:30:27 -0500 > > > > > On Fri, 2006-13-01 at 09:27 +0100, Per Liden wrote: > > > > Increasing the module ref count at registration will block the module > > > > from > > > > ever being unloaded. In fact, genetlink should not care about the owner > > > > at > > > > all. This patch removes the owner field from the struct registered with > > > > genetlink. > > > > > > > > Signed-off-by: Per Liden <[EMAIL PROTECTED]> > > > > > > Signed-off-by: Jamal Hadi Salim <[EMAIL PROTECTED]> > > > > > > BTW, thinking in the background we may in the future need the owner > > > field for users of those modules (not for gennetlink to increment) but > > > lets worry about that in the future. > > > > I don't have a copy of this patch in my inbox, can someone > > resend it to me? Thanks. > > Resending. Thank you. I'll apply this. There might be a better way to handle this, but for now this is fine. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] genetlink: don't touch module ref count
From: Per Liden <[EMAIL PROTECTED]> Date: Fri, 13 Jan 2006 21:56:53 +0100 (CET) > Signed-off-by: Per Liden <[EMAIL PROTECTED]> BTW, don't do this, it should be a perfectly functioning email address so that people can contact you. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] genetlink: don't touch module ref count
On Fri, 13 Jan 2006, David S. Miller wrote: > From: Jamal Hadi Salim <[EMAIL PROTECTED]> > Date: Fri, 13 Jan 2006 09:30:27 -0500 > > > On Fri, 2006-13-01 at 09:27 +0100, Per Liden wrote: > > > Increasing the module ref count at registration will block the module from > > > ever being unloaded. In fact, genetlink should not care about the owner at > > > all. This patch removes the owner field from the struct registered with > > > genetlink. > > > > > > Signed-off-by: Per Liden <[EMAIL PROTECTED]> > > > > Signed-off-by: Jamal Hadi Salim <[EMAIL PROTECTED]> > > > > BTW, thinking in the background we may in the future need the owner > > field for users of those modules (not for gennetlink to increment) but > > lets worry about that in the future. > > I don't have a copy of this patch in my inbox, can someone > resend it to me? Thanks. Resending. /Per [PATCH] genetlink: don't touch module ref count Increasing the module ref count at registration will block the module from ever being unloaded. In fact, genetlink should not care about the owner at all. This patch removes the owner field from the struct registered with genetlink. Signed-off-by: Per Liden <[EMAIL PROTECTED]> Signed-off-by: Jamal Hadi Salim <[EMAIL PROTECTED]> --- include/net/genetlink.h |1 - net/netlink/genetlink.c |7 --- net/tipc/netlink.c |1 - 3 files changed, 0 insertions(+), 9 deletions(-) 4647eec00ae3dc1be979f5387d766529cb64e29c diff --git a/include/net/genetlink.h b/include/net/genetlink.h index c5b96b2..805de50 100644 --- a/include/net/genetlink.h +++ b/include/net/genetlink.h @@ -22,7 +22,6 @@ struct genl_family charname[GENL_NAMSIZ]; unsigned intversion; unsigned intmaxattr; - struct module * owner; struct nlattr **attrbuf;/* private */ struct list_headops_list; /* private */ struct list_headfamily_list;/* private */ diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c index 3b13784..4ae1538 100644 --- a/net/netlink/genetlink.c +++ b/net/netlink/genetlink.c @@ -222,11 +222,6 @@ int genl_register_family(struct genl_fam goto errout_locked; } - if (!try_module_get(family->owner)) { - err = -EBUSY; - goto errout_locked; - } - if (family->id == GENL_ID_GENERATE) { u16 newid = genl_generate_id(); @@ -283,7 +278,6 @@ int genl_unregister_family(struct genl_f INIT_LIST_HEAD(&family->ops_list); genl_unlock(); - module_put(family->owner); kfree(family->attrbuf); genl_ctrl_event(CTRL_CMD_DELFAMILY, family); return 0; @@ -535,7 +529,6 @@ static struct genl_family genl_ctrl = { .name = "nlctrl", .version = 0x1, .maxattr = CTRL_ATTR_MAX, - .owner = THIS_MODULE, }; static int __init genl_init(void) diff --git a/net/tipc/netlink.c b/net/tipc/netlink.c index 6fe95ac..19b3f40 100644 --- a/net/tipc/netlink.c +++ b/net/tipc/netlink.c @@ -72,7 +72,6 @@ static struct genl_family family = { .version = TIPC_GENL_VERSION, .hdrsize = TIPC_GENL_HDRLEN, .maxattr = 0, - .owner = THIS_MODULE, }; static struct genl_ops ops = { -- 1.0.GIT - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] genetlink: don't touch module ref count
From: Jamal Hadi Salim <[EMAIL PROTECTED]> Date: Fri, 13 Jan 2006 09:30:27 -0500 > On Fri, 2006-13-01 at 09:27 +0100, Per Liden wrote: > > Increasing the module ref count at registration will block the module from > > ever being unloaded. In fact, genetlink should not care about the owner at > > all. This patch removes the owner field from the struct registered with > > genetlink. > > > > Signed-off-by: Per Liden <[EMAIL PROTECTED]> > > Signed-off-by: Jamal Hadi Salim <[EMAIL PROTECTED]> > > BTW, thinking in the background we may in the future need the owner > field for users of those modules (not for gennetlink to increment) but > lets worry about that in the future. I don't have a copy of this patch in my inbox, can someone resend it to me? Thanks. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] genetlink: don't touch module ref count
Hi Thomas, On Fri, 13 Jan 2006, Thomas Graf wrote: > * Per Liden <[EMAIL PROTECTED]> 2006-01-13 09:27 > > Increasing the module ref count at registration will block the module from > > ever being unloaded. In fact, genetlink should not care about the owner at > > all. This patch removes the owner field from the struct registered with > > genetlink. > > Why shouldn't it care? When registering you hand over control of your > family and callback structures which may not be accessed after the > module has been unloaded. Right, but to make sure the structure isn't accessed after the module is unloaded all the module has to do is to unregister it when its exit function is called. If the module doesn't do that, then that's a bug in the module which should be fixed. The genl_sem semaphore makes sure the module can't unregister its structure when it's actually in use (i.e. when a genetlink command for that family is in progress). > If you want to have your module unloaded, unregister your family first. No, think about it. This is exactly where the "deadlock" happens. To unregister your family you need to get a call on your module's exit function. However, that exit function will _never_ be called by the kernel since it will simply not allow that when the modules ref count is > 0. Hence, you will never be able to unload that module ever again. /Per - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] genetlink: don't touch module ref count
* Per Liden <[EMAIL PROTECTED]> 2006-01-13 09:27 > Increasing the module ref count at registration will block the module from > ever being unloaded. In fact, genetlink should not care about the owner at > all. This patch removes the owner field from the struct registered with > genetlink. Why shouldn't it care? When registering you hand over control of your family and callback structures which may not be accessed after the module has been unloaded. If you want to have your module unloaded, unregister your family first. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] genetlink: don't touch module ref count
On Fri, 2006-13-01 at 09:27 +0100, Per Liden wrote: > Increasing the module ref count at registration will block the module from > ever being unloaded. In fact, genetlink should not care about the owner at > all. This patch removes the owner field from the struct registered with > genetlink. > > Signed-off-by: Per Liden <[EMAIL PROTECTED]> Signed-off-by: Jamal Hadi Salim <[EMAIL PROTECTED]> BTW, thinking in the background we may in the future need the owner field for users of those modules (not for gennetlink to increment) but lets worry about that in the future. cheers, jamal - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] genetlink: don't touch module ref count
Increasing the module ref count at registration will block the module from ever being unloaded. In fact, genetlink should not care about the owner at all. This patch removes the owner field from the struct registered with genetlink. Signed-off-by: Per Liden <[EMAIL PROTECTED]> --- include/net/genetlink.h |1 - net/netlink/genetlink.c |7 --- net/tipc/netlink.c |1 - 3 files changed, 0 insertions(+), 9 deletions(-) 4647eec00ae3dc1be979f5387d766529cb64e29c diff --git a/include/net/genetlink.h b/include/net/genetlink.h index c5b96b2..805de50 100644 --- a/include/net/genetlink.h +++ b/include/net/genetlink.h @@ -22,7 +22,6 @@ struct genl_family charname[GENL_NAMSIZ]; unsigned intversion; unsigned intmaxattr; - struct module * owner; struct nlattr **attrbuf;/* private */ struct list_headops_list; /* private */ struct list_headfamily_list;/* private */ diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c index 3b13784..4ae1538 100644 --- a/net/netlink/genetlink.c +++ b/net/netlink/genetlink.c @@ -222,11 +222,6 @@ int genl_register_family(struct genl_fam goto errout_locked; } - if (!try_module_get(family->owner)) { - err = -EBUSY; - goto errout_locked; - } - if (family->id == GENL_ID_GENERATE) { u16 newid = genl_generate_id(); @@ -283,7 +278,6 @@ int genl_unregister_family(struct genl_f INIT_LIST_HEAD(&family->ops_list); genl_unlock(); - module_put(family->owner); kfree(family->attrbuf); genl_ctrl_event(CTRL_CMD_DELFAMILY, family); return 0; @@ -535,7 +529,6 @@ static struct genl_family genl_ctrl = { .name = "nlctrl", .version = 0x1, .maxattr = CTRL_ATTR_MAX, - .owner = THIS_MODULE, }; static int __init genl_init(void) diff --git a/net/tipc/netlink.c b/net/tipc/netlink.c index 6fe95ac..19b3f40 100644 --- a/net/tipc/netlink.c +++ b/net/tipc/netlink.c @@ -72,7 +72,6 @@ static struct genl_family family = { .version = TIPC_GENL_VERSION, .hdrsize = TIPC_GENL_HDRLEN, .maxattr = 0, - .owner = THIS_MODULE, }; static struct genl_ops ops = { -- 1.0.GIT - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html