On Tue, Sep 01, 2015 at 05:31:44PM -0700, Martin KaFai Lau wrote: > On Tue, Sep 01, 2015 at 03:38:36PM -0700, Eric Dumazet wrote: > > On Tue, 2015-09-01 at 15:25 -0700, Martin KaFai Lau wrote: > > > On Tue, Sep 01, 2015 at 02:26:58PM -0700, Eric Dumazet wrote: > > > > On Tue, 2015-09-01 at 13:55 -0700, Martin KaFai Lau wrote: > > > > > On Tue, Sep 01, 2015 at 01:14:20PM -0700, Eric Dumazet wrote: > > > > > > It should not be a problem. refcnt is taken when/if necessary (skb > > > > > > queued on a qdisc for example) > > > > > > > > > > > > We have other uses of skb_dst_set_noref() > > > > > > > > > > > > Please describe the problem ? > > > > > The current ip6_tnl_dst_get() does not take the dst refcnt. > > > > > > > > > > If the dst is released after ip6_tnl_dst_get() and before > > > > > skb_dst_set_noref(), would it cause an issue? > > > > > > > > We are under rcu here, and a dst in a cache is protected by RCU by > > > > definition. > > > > > > > > skb_dst_set_noref() has following debugging clause, does it trigger for > > > > you ? > > > > > > > > WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held()); > > > No. I did not see this. > > > > > > I am probably missing something. Do you mean the rcu can > > > protect the followings: > > > > > > > > > ip6_tnl_dst_get() > > > dst_release() > > > dst_free() /* refcnt is 0 */ > > > skb_dst_set_noref() > > > > > > > Yes, this is protected by normal rcu rules. > > > > dst wont be freed until all cpus exit their rcu read sections. > For DST_NOCACHE, like: > > rcu_read_lock() > > ip6_tnl_dst_get() > dst_release() /* refcnt is 0 */ > =>call_rcu(dst_destroy) > skb_dst_set_noref() > __dev_queue_xmit() > =>skb_dst_force() > =>__dev_xmit_skb() > =>q->enqueue() > > rcu_read_unlock() > /* Here, I am holding a dst refcnt but > * the dst is already in the next > * rcu destroy cycle? > */ I look a closer look at dst_rcu_free() and your commit pointers. I can see your point for DST_NOCACHE.
However, dst_free() for not DST_NOCACHE is still an issue, I think. -- 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
