Sorry - this is the other half of my reply that I accidentally deleted
before sending...
 
> +static void flush_tx_skbs(struct ieee80211_sub_if_data *sdata)
> +{
> +     struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
> +     struct mesh_tx_queue *tx_node;
> +
> +     spin_lock_bh(&ifmsh->mesh_tx_queue_lock);
> +
> +     /* Note that this check is important because of the two-
> stage
> +      * way that ieee80211_if_mesh is initialized.
> +      */

I think you should fix that rather than work around it. If this is
called with iftype != mesh then this is super problematic anyway, since
ifmsh->tx_queue_len would alias some other variable (there's a union).

> +     if (ifmsh->tx_queue_len > 0) {
> +             mhwmp_dbg(sdata, "flushing %d skbs", ifmsh-
> >tx_queue_len);
> +
> +             while (!list_empty(&ifmsh->tx_queue.list)) {
> +                     tx_node = list_last_entry(&ifmsh-
> >tx_queue.list,
> +                                               struct
> mesh_tx_queue, list);
> +                     kfree_skb(tx_node->skb);
> +                     list_del(&tx_node->list);
> +                     kfree(tx_node);
> +             }
> +             ifmsh->tx_queue_len = 0;
> +     }
> +
> +     spin_unlock_bh(&ifmsh->mesh_tx_queue_lock);
> +}

All of this also gets *vastly* simpler if it's just skb_queue_purge()
:)

> +     spin_lock_bh(&ifmsh->mesh_tx_queue_lock);
> +
> +     list_for_each_entry(tx_node, &ifmsh->tx_queue.list, list) {
> +             ieee80211_tx_skb(sdata, tx_node->skb);
> +     }

I don't think you should hold the lock across _tx_skb(), ISTR problems
with that - particularly with the STA lock, so this might be OK, but it
might also cause lock ordering issues. It's easy to avoid anyway, so
better not to do it.

johannes

Reply via email to