On Friday 04 March 2016 16:50:43 Sven Eckelmann wrote:
On Friday 04 March 2016 16:21:32 Andrew Lunn wrote:
Hi
I'm sometimes getting a crash after removing a hard interface when the batadv_send_outstanding_bat_org_packet() is called in a work queue. It calls
static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff,
int packet_len, unsigned longsend_time, bool direct_link,
struct batadv_hard_iface*if_incoming, struct batadv_hard_iface *if_outgoing, int own_packet) {
struct batadv_priv *bat_priv = netdev_priv(if_incoming->soft_iface);struct batadv_forw_packet *forw_packet_aggr;
unsigned char *skb_buff; unsigned int skb_size; if (!kref_get_unless_zero(&if_incoming->refcount)) return; if (!kref_get_unless_zero(&if_outgoing->refcount)) goto out_free_incoming;Given that we have:
static inline void batadv_hardif_put(struct batadv_hard_iface *hard_iface) {
kref_put(&hard_iface->refcount, batadv_hardif_release);}
does using kref_get_unless_zero() make sense? If it is zero, hasn't it been freed by the kref_put that set it to zero?
Maybe it would be easier to understand when this would be replaced with kref_get and the if_outgoing loop in batadv_iv_ogm_schedule would be replaced with:
rcu_read_lock(); list_for_each_entry_rcu(tmp_hard_iface, &batadv_hardif_list, list) { if (tmp_hard_iface->soft_iface != hard_iface->soft_iface) continue;
/* make sure only still valid interfaces are used in queue */ if (!kref_get_unless_zero(&tmp_hard_iface->refcount)) continue; batadv_iv_ogm_queue_add(bat_priv, *ogm_buff, *ogm_buff_len, hard_iface, tmp_hard_iface, 1, send_time); batadv_hardif_put(tmp_hard_iface); } rcu_read_unlock();
Sorry for being in noisy-mail mode. I will stop sending mails for today.
Kind regards, Sven