Re: [PATCH/RFC] New module refcounting for net_proto_family

[Rusty Russell]

In message <[e-mail]> you write:
> At 04:30 PM 2/20/2003, Rusty Russell wrote:
> >Yes, but in practical terms it's probably going to fork a child with
> >that socket.
> But it will also be killed.

Once they track it down 8).

> >> >I think it can be argued both ways, honestly.
> >> Yep. And I'd argue in for of module_get() :)
> >
> >My only real insistence in this is that such an interface be called
> >__try_module_get(), because the "__" warn people that it's a "you'd
> >better know *exactly* what you are doing", even though the "try" is a
> >bit of a misnomer.
> Yeah, I think 'try' is definitely be a misnomer in this case.
> How about something like this ?

No, I definitely want the name __try_module_get.  Sure, it's a
misnomer in one sense, which will hopefully scare off people looking
for an easy way out.  OTOH, it accurately reflects "you should be
using try_module_get but you have special circumstances" more
eloquently than any comment ever would.  Especially since there are
only a handful of places where it is appropriate.

I think a CONFIG option for checking is overkill: better is to grep
each kernel for __try_module_get() being added and make sure the damn
thing doesn't spread 8)

Cheers,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Name: __try_module_get
Author: Rusty Russell
Status: Trivial
Depends: 

D: Introduces __try_module_get for places where we know we already hold
D: a reference and ignoring the fact that the module is being "rmmod --wait"ed
D: is simpler.

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .12219-linux-2.5.62-bk3/include/linux/module.h .12219-linux-2.5.62-bk3.updated/include/linux/module.h
--- .12219-linux-2.5.62-bk3/include/linux/module.h	2003-02-21 10:32:28.000000000 +1100
+++ .12219-linux-2.5.62-bk3.updated/include/linux/module.h	2003-02-24 11:46:13.000000000 +1100
@@ -292,17 +292,23 @@ void symbol_put_addr(void *addr);
 #define local_dec(x) atomic_dec(x)
 #endif
 
+/* Sometimes we know we already have a refcount, and it's easier not
+   to handle the error case (which only happens with rmmod --wait). */
+static inline void __try_module_get(struct module *module)
+{
+	local_inc(&module->ref[get_cpu()].count);
+	put_cpu();
+}
+
 static inline int try_module_get(struct module *module)
 {
 	int ret = 1;
 
 	if (module) {
-		unsigned int cpu = get_cpu();
 		if (likely(module_is_live(module)))
-			local_inc(&module->ref[cpu].count);
+			__try_module_get(module);
 		else
 			ret = 0;
-		put_cpu();
 	}
 	return ret;
 }
@@ -327,6 +332,9 @@ static inline int try_module_get(struct 
 static inline void module_put(struct module *module)
 {
 }
+static inline void __try_module_get(struct module *module)
+{
+}
 #define symbol_put(x) do { } while(0)
 #define symbol_put_addr(p) do { } while(0)
 
@@ -381,6 +389,10 @@ static inline int module_text_address(un
 #define symbol_put(x) do { } while(0)
 #define symbol_put_addr(x) do { } while(0)
 
+static inline void __try_module_get(struct module *module)
+{
+}
+
 static inline int try_module_get(struct module *module)
 {
 	return 1;