All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: linux-kernel@vger.kernel.org
Subject: Re: Using list_for_each_entry() in place of list_for_each_entry_rcu() ?
Date: Wed, 5 Nov 2014 08:56:55 -0800	[thread overview]
Message-ID: <20141105165655.GG5718@linux.vnet.ibm.com> (raw)
In-Reply-To: <201411042018.FBG60918.JOMOLOtHFQSVFF@I-love.SAKURA.ne.jp>

On Tue, Nov 04, 2014 at 08:18:03PM +0900, Tetsuo Handa wrote:
> Paul E. McKenney wrote:
> > But if you only ever add it to the list that one time, then the
> > list_for_each_entry_rcu() could become list_for_each_entry(), and
> > rcu_read_lock() and rcu_read_unlock() are not needed.  Again, this
> > assumes that the memory is never reused after being removed from the list.
> 
> That is what I wanted to confirm. v1, v2, v3 are added to the list only
> once, and mymodule.ko containing v1, v2, v3 are not unload-able.

Good enough, then.  ;-)

> > Note that in this case module unload followed by module reload is tricky.
> > You have to "wait long enough" between unload and load.  Or you need an
> > orderly teardown of some sort.
> 
> Yes, I'm aware of that.
> 
> > > Assumptions are:
> > > 
> > >   (1) v1, v2, v3 are statically allocated variables inside module,
> > >       while my_lock, my_list, add_entry(), del_entry(), reader()
> > >       are built-in.
> > 
> > When you say that my_lock and my_list are built-in, you mean that they
> > are defined in the base kernel rather than in the module?  (I was assuming
> > that they were defined in the module.)
> 
> I meant built-in as built into vmlinux. That is, my_lock, my_list,
> add_entry(), del_entry(), reader() are defined in vmlinux , and v1, v2, v3
> are defined in mymodule.ko .

OK, even more straightforward, then.

> > >   (2) v1, v2, v3 are added to my_list only once upon module load
> > > 
> > >   (3) v1, v2, v3 might be removed from my_list some time later after
> > >       module was loaded
> > 
> > Again, module unload followed by module load will be a bit dicey.  Other
> > than that, it should work.
> > 
> > 							Thanx, Paul
> > 
> 
> If someone really needs to implement unload-able modules, he/she can
> split such modules into non unload-able component and unload-able
> component.

Yep, that works also.

							Thanx, Paul

> -------- non unload-able component --------
> static DEFINE_SRCU(my_srcu_lock);
> static int (*do_getvalue)(void);
> 
> static int mywrapper_getvalue(void)
> {
>     const int idx = srcu_read_lock(&my_srcu_lock);
>     const typeof(do_getvalue) func = do_getvalue;
>     const int ret = func ? func() : 0;
>     srcu_read_unlock(&my_srcu_lock, idx);
>     return ret;
> }
> 
> void mywrapper_register(typeof(do_getvalue) func)
> {
>     do_getvalue = func;
> }
> EXPORT_SYMBOL_GPL(mywrapper_register);
> 
> void mywrapper_unregister(void)
> {
>     do_getvalue = NULL;
>     synchronize_srcu(&my_srcu_lock);
> }
> EXPORT_SYMBOL_GPL(mywrapper_unregister);
> 
> struct my_struct {
>     struct list_head list;
>     const int (*func)(void);
> } v1 = { .func = mywrapper_getvalue };
> 
> static int __init mywrapper_init(void)
> {
>     add_entry(&v1);
>     return 0;
> }
> module_init(mywrapper_init);
> -------- non unload-able component --------
> 
> -------- unload-able component --------
> static int do_getvalue(void)
> {
>     return (...snipped...);
> }
> 
> static int __init mymodule_init(void)
> {
>     mywrapper_register(do_getvalue);
>     return 0;
> }
> module_init(mymodule_init);
> 
> static void mymodule_exit(void)
> {
>     mywrapper_unregister();
> }
> module_exit(mymodule_exit);
> -------- unload-able component --------
> 
> Thank you.
> 


      reply	other threads:[~2014-11-05 16:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-01 11:59 Using list_for_each_entry() in place of list_for_each_entry_rcu() ? Tetsuo Handa
2014-11-04  1:54 ` Paul E. McKenney
2014-11-04 11:18   ` Tetsuo Handa
2014-11-05 16:56     ` Paul E. McKenney [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141105165655.GG5718@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.