Hannes Frederic Sowa <han...@stressinduktion.org> wrote: > > > > @@ -936,7 +936,9 @@ static void ipmr_cache_resolve(struct net *net, > > > > struct mr_table *mrt, > > > > > > > > rtnl_unicast(skb, net, NETLINK_CB(skb).portid); > > > > } else { > > > > + preempt_disable(); > > > > ip_mr_forward(net, mrt, skb, c, 0); > > > > + preempt_enable(); > > > > } > > > > } > > > > } > > > > > > I do not believe this fix is correct. > > > > Yes, sorry. I should have suggested local_bh_disable instead. > > > > > Better replace the > > > IP_INC_STATS_BH() by IP_INC_STATS() > > > > > > and IP_ADD_STATS_BH() by IP_ADD_STATS() > > > > Hmm, whats the rationale for this? > > > > Note that IP_ADD_STATS_BH in question is unconditional (not in > > error path). It seems that its virtually always called from softirq > > except in the setsockopt case. > > The naming of the functions is bad if you compare them to e.g. > spin_lock_bh. > > STATS_BH can only be used from bottom half and the normal ones (without > _BH) can be called from everywhere. It is a common pattern in the > kernel. > > Eric's proposal is correct.
Yes, its correct but it results in 4 additonal bh on/off calls for the common case, hence my question. Moving the one ip_mr_forward into bh-off keeps the bh-disable thing in the setsockopt path. -- 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