Re: [PATCH] genetlink: don't touch module ref count

2006-01-13 Thread Per Liden
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

2006-01-13 Thread David S. Miller
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

2006-01-13 Thread David S. Miller
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

2006-01-13 Thread Per Liden
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

2006-01-13 Thread David S. Miller
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

2006-01-13 Thread Per Liden
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

2006-01-13 Thread Thomas Graf
* 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

2006-01-13 Thread Jamal Hadi Salim
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

2006-01-13 Thread Per Liden
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