All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
To: Cong Wang <xiyou.wangcong@gmail.com>
Cc: Santosh Shilimkar <santosh.shilimkar@oracle.com>,
	Linux Kernel Network Developers <netdev@vger.kernel.org>,
	rds-devel@oss.oracle.com
Subject: Re: [Patch net] rds: mark bound socket with SOCK_RCU_FREE
Date: Mon, 10 Sep 2018 20:24:23 -0400	[thread overview]
Message-ID: <20180911002423.GL4668@oracle.com> (raw)
In-Reply-To: <CAM_iQpXcydKdTyT25F5Bbk7y9BMJLTKMT42o3J0bbuodjXjetA@mail.gmail.com>

On (09/10/18 17:16), Cong Wang wrote:
> >
> > On (09/10/18 16:51), Cong Wang wrote:
> > >
> > >         __rds_create_bind_key(key, addr, port, scope_id);
> > > -       rs = rhashtable_lookup_fast(&bind_hash_table, key, ht_parms);
> > > +       rcu_read_lock();
> > > +       rs = rhashtable_lookup(&bind_hash_table, key, ht_parms);
> > >         if (rs && !sock_flag(rds_rs_to_sk(rs), SOCK_DEAD))
> > >                 rds_sock_addref(rs);
> > >         else
> > >                 rs = NULL;
> > > +       rcu_read_unlock();
> >
> > aiui, the rcu_read lock/unlock is only useful if the write
> > side doing destructive operations  does something to make sure readers
> > are done before doing the destructive opertion. AFAIK, that does
> > not exist for rds socket management today
> 
> That is exactly why we need it here, right?

Maybe I am confused, what exactly is the patch you are proposing?

Does it have the SOCK_RCU_FREE change? 
Does it have the rcu_read_lock you have above? 
Where is the call_rcu?

> Hmm, so you are saying synchronize_rcu() is kinda more correct
> than call_rcu()??  


I'm not saying that, I'm asking "what exactly is the patch
you are proposing?" The only one on record is 
   http://patchwork.ozlabs.org/patch/968282/
which does not have either synchronize_rcu or call_rcu.

> I never hear this before, would like to know why.

Please post precise patches first.

Thanks.

  reply	other threads:[~2018-09-11  5:21 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-10 22:24 [Patch net] rds: mark bound socket with SOCK_RCU_FREE Cong Wang
2018-09-10 22:34 ` Sowmini Varadhan
2018-09-10 22:43 ` Santosh Shilimkar
2018-09-10 23:30   ` Sowmini Varadhan
2018-09-10 23:51     ` Cong Wang
2018-09-11  0:04       ` Sowmini Varadhan
2018-09-11  0:16         ` Cong Wang
2018-09-11  0:24           ` Sowmini Varadhan [this message]
2018-09-11  0:39             ` Cong Wang
2018-09-11  0:26           ` Santosh Shilimkar
2018-09-11  0:45             ` Cong Wang
2018-09-11  0:56               ` Santosh Shilimkar
2018-09-11  0:59                 ` Cong Wang

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=20180911002423.GL4668@oracle.com \
    --to=sowmini.varadhan@oracle.com \
    --cc=netdev@vger.kernel.org \
    --cc=rds-devel@oss.oracle.com \
    --cc=santosh.shilimkar@oracle.com \
    --cc=xiyou.wangcong@gmail.com \
    /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.