On Sat, 2015-12-05 at 20:37 -0500, Dave Jones wrote:
> On Sat, Dec 05, 2015 at 05:13:06PM -0800, Eric Dumazet wrote:
>
> > > diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> > > index acb45b8c2a9d..7081183f4d9f 100644
> > > --- a/net/sctp/ipv6.c
> > > +++ b/net/sctp/ipv6.c
> > > @@ -328,7 +328,9 @@ static void sctp_v6_get_dst(struct sctp_transport
> *t, union sctp_addr *saddr,
> > > if (baddr) {
> > > fl6->saddr = baddr->v6.sin6_addr;
> > > fl6->fl6_sport = baddr->v6.sin6_port;
> > > + rcu_read_lock();
> > > final_p = fl6_update_dst(fl6, rcu_dereference(np->opt),
> &final);
> > > + rcu_read_unlock();
> > > dst = ip6_dst_lookup_flow(sk, fl6, final_p);
> > > }
> > >
> >
> > Hmm, better use :
> >
> > diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> > index acb45b8c2a9d..d28c0b4c9128 100644
> > --- a/net/sctp/ipv6.c
> > +++ b/net/sctp/ipv6.c
> > @@ -323,14 +323,13 @@ static void sctp_v6_get_dst(struct sctp_transport
> *t, union sctp_addr *saddr,
> > }
> > }
> > }
> > - rcu_read_unlock();
> > -
> > if (baddr) {
> > fl6->saddr = baddr->v6.sin6_addr;
> > fl6->fl6_sport = baddr->v6.sin6_port;
> > final_p = fl6_update_dst(fl6, rcu_dereference(np->opt), &final);
> > dst = ip6_dst_lookup_flow(sk, fl6, final_p);
> > }
> > + rcu_read_unlock();
> >
> > out:
> > if (!IS_ERR_OR_NULL(dst)) {
>
> I looked at that option first, but decided to mirror the other use of
> fl6_update_dst.
>
> It looks like your solution would work too, so I'm not against it, but..
> For my own understanding, why is this better? Just to cut down on the
> number of repeated lock/unlocks in the same function? Or is there some
> semantic I'm missing in the earlier lock/unlock section that's somehow
> related to the np->opt ?
This was my intent when cooking commit c836a8ba93869d6a0
I have no idea how I missed to move the rcu_read_lock()
Yes, there is no need to have too many rcu_read_lock()/unlock() all
around the places. Extending the existing section is good enough.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
