From: Vlad Yasevich <vladislav.yasevich@hp•com>
To: Sridhar Samudrala <sri@us•ibm.com>
Cc: netdev@vger•kernel.org, lksctp-developers@lists•sourceforge.net
Subject: Re: [RFC v2 PATCH 1/2] SCTP: Add RCU synchronization around sctp_localaddr_list
Date: Thu, 13 Sep 2007 09:46:24 -0400 [thread overview]
Message-ID: <46E93F30.5040909@hp.com> (raw)
In-Reply-To: <1189638236.27182.12.camel@w-sridhar2.beaverton.ibm.com>
Hi Sridhar
Sridhar Samudrala wrote:
> Vlad,
>
> few minor comments inline.
> otherwise, looks good.
>
> Thanks
> Sridhar
>
>> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
>> index f8aa23d..54ff472 100644
>> --- a/net/sctp/ipv6.c
>> +++ b/net/sctp/ipv6.c
>> @@ -77,13 +77,18 @@
>>
>> #include <asm/uaccess.h>
>>
>> -/* Event handler for inet6 address addition/deletion events. */
>> +/* Event handler for inet6 address addition/deletion events.
>> + * This even is part of the atomic notifier call chain
>> + * and thus happens atomically and can NOT sleep. As a result
>> + * we can't and really don't need to add any locks to guard the
>> + * RCU.
>> + */
>
> Now that we are adding a spin_lock, the above comment is not valid.
> It should be fixed saying that we still need a lock because we use the
> same list for both inet and inet6 address events and they can happen in
> parallel.
Yes, I forgot to fix this comment. Will do.
>> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
>> index e98579b..4688559 100644
>> --- a/net/sctp/protocol.c
>> +++ b/net/sctp/protocol.c
>> @@ -153,6 +153,8 @@ static void sctp_v4_copy_addrlist(struct list_head *addrlist,
>> addr->a.v4.sin_family = AF_INET;
>> addr->a.v4.sin_port = 0;
>> addr->a.v4.sin_addr.s_addr = ifa->ifa_local;
>> + addr->valid = 1;
>> + INIT_RCU_HEAD(&addr->rcu);
>
> This has nothing to do with this patch, but i noticed that
> INIT_LIST_HEAD(&addr->list) is missing here when comparing with
> earlier v6 version of this routine.
Hmm... I thought it looked a little different, but didn't pay too much
attention to it. I'll add a follow-on patch to fix this.
Thanks
-vlad
next prev parent reply other threads:[~2007-09-13 13:46 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-09-12 19:46 [RFC v2 PATCH 0/2] Add RCU locking to SCTP address management Vlad Yasevich
2007-09-12 19:46 ` [RFC v2 PATCH 1/2] SCTP: Add RCU synchronization around sctp_localaddr_list Vlad Yasevich
2007-09-12 22:26 ` Paul E. McKenney
2007-09-12 23:03 ` Sridhar Samudrala
2007-09-13 13:46 ` Vlad Yasevich [this message]
2007-09-12 19:46 ` [RFC v2 PATCH 2/2] SCTP: Convert bind_addr_list locking to RCU Vlad Yasevich
2007-09-12 20:59 ` [RFC v2 PATCH 0/2] Add RCU locking to SCTP address management Vlad Yasevich
2007-09-12 21:03 ` [RFC v3 PATCH 2/21] SCTP: Convert bind_addr_list locking to RCU Vlad Yasevich
2007-09-12 22:33 ` Paul E. McKenney
2007-09-13 17:59 ` Sridhar Samudrala
2007-09-13 18:15 ` Vlad Yasevich
2007-09-13 19:33 ` [Lksctp-developers] " Vlad Yasevich
2007-09-13 19:56 ` Sridhar Samudrala
2007-09-13 20:14 ` Vlad Yasevich
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=46E93F30.5040909@hp.com \
--to=vladislav.yasevich@hp$(echo .)com \
--cc=lksctp-developers@lists$(echo .)sourceforge.net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=sri@us$(echo .)ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox