On Wed, 2012-09-05 at 20:23 +0300, Eliad Peller wrote:
> +++ b/net/mac80211/iface.c
> @@ -774,6 +774,10 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
> * frames at this very time on another CPU.
> */
> synchronize_rcu();
> +
> + /* flush any remaining cleanup work */
> + flush_workqueue(local->workqueue);
I have a feeling we need rcu_barrier() here instead of synchronize_rcu()
now to guarantee that all the potentially pending call_rcu() functions
from the sta_info_flush() above have actually completed and queued the
work items that we want to flush here. Right? That's a bit unfortunate,
but I guess the best way of handling it and ifdown should be rare anyway
compared to everything else.
I think it'd also be good to make the comment more verbose and explain
which cleanup work.
> +static void free_sta_work(struct work_struct *wk)
> +{
> + struct sta_info *sta = container_of(wk, struct sta_info, free_sta_wk);
> + int ac, i;
> + struct tid_ampdu_tx *tid_tx;
> + struct ieee80211_sub_if_data *sdata = sta->sdata;
> + struct ieee80211_local *local = sdata->local;
> +
> + /*
> + * At this point, after we wait for an RCU grace period,
> + * neither mac80211 nor the driver can reference this
> + * sta struct any more except by still existing timers
> + * associated with this station that we clean up below.
> + */
Should probably rewrite the comment a bit to indicate that we have in
fact waited since we get called via call_rcu().
> + cancel_work_sync(&sta->drv_unblock_wk);
This will deadlock now, but cancel_work() will be safe without the sync
since the workqueue is single-threaded.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
