On Wed, 2017-09-20 at 18:09 -0700, Wei Wang wrote:
> > Thanks very much Pawel for the feedback.
> >
> > I was looking into the code (specifically IPv4 part) and found that in
> > free_fib_info_rcu(), we call free_nh_exceptions() without holding the
> > fnhe_lock. I am wondering if that could cause some race condition on
> > fnhe->fnhe_rth_input/output so a double call on dst_dev_put() on the
> > same dst could be happening.
> >
> > But as we call free_fib_info_rcu() only after the grace period, and
> > the lookup code which could potentially modify
> > fnhe->fnhe_rth_input/output all holds rcu_read_lock(), it seems
> > fine...
> >
>
> Hi Pawel,
>
> Could you try the following debug patch on top of net-next branch and
> reproduce the issue check if there are warning msg showing?
>
> diff --git a/include/net/dst.h b/include/net/dst.h
> index 93568bd0a352..82aff41c6f63 100644
> --- a/include/net/dst.h
> +++ b/include/net/dst.h
> @@ -271,7 +271,7 @@ static inline void dst_use_noref(struct dst_entry
> *dst, unsigned long time)
> static inline struct dst_entry *dst_clone(struct dst_entry *dst)
> {
> if (dst)
> - atomic_inc(&dst->__refcnt);
> + dst_hold(dst);
> return dst;
> }
>
> Thanks.
> Wei
>
Yes, we believe skb_dst_force() and skb_dst_force_safe() should be
unified (to the 'safe' version)
We no longer have gc to protect from 0 -> 1 transition of dst refcount.
