From: Stanislaw Gruszka <sgruszka@redhat.com>
To: greearb@candelatech.com
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH] mac80211: Clean up work-queues on disassociation.
Date: Mon, 25 Feb 2013 11:20:21 +0100 [thread overview]
Message-ID: <20130225102020.GA1735@redhat.com> (raw)
In-Reply-To: <1361326267-16847-1-git-send-email-greearb@candelatech.com>
On Tue, Feb 19, 2013 at 06:11:07PM -0800, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> The monitor_work and beacon_connection_loss_work items were
> not being canceled on disassociation (and not on deletion
> either). This leads to work-items trying to run after memory
> has been deleted.
>
> I could not find a cleaner way to do this because the
> cancel_work_sync for these items must be done outside
> of the ifmgd->mtx.
>
> In addition, re-order the quiesce code so that timers are
> always stopped before work-items are flushed. This was
> not the problem I saw, but I think it may still be more
> correct.
I think this patch is quite complicated and simpler solution
can be used. We stop timers on disassociate, and since
we nullify ifmgd->associated under ifmgd->mtx, work procedures
will perform no action. The only thing we should care are works
queued to workqueue internals, and stop them before we remove
the interface, to stop workqueue code to use our freed
ifmgd->*_work data.
Regarding quiesce, I'm working on suspend/resume changes where this
function become not necessary, so I'll remove it.
Below is alternative fix proposition for the problem.
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index bdddb0b..15a24214 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -3992,6 +3992,17 @@ void ieee80211_mgd_stop(struct ieee80211_sub_if_data *sdata)
{
struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
+ /*
+ * We canceled timers during disassoc, but works still can be pending.
+ * Even if we they do not perform action when unassociated, we should
+ * assure we stop them, before freeing resources.
+ */
+ cancel_work_sync(&ifmgd->request_smps_work);
+ cancel_work_sync(&ifmgd->monitor_work);
+ cancel_work_sync(&ifmgd->beacon_connection_loss_work);
+ cancel_work_sync(&ifmgd->csa_connection_drop_work);
+ cancel_work_sync(&ifmgd->chswitch_work);
+
mutex_lock(&ifmgd->mtx);
if (ifmgd->assoc_data)
ieee80211_destroy_assoc_data(sdata, false);
--
1.7.11.7
next prev parent reply other threads:[~2013-02-25 10:20 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-20 2:11 [PATCH] mac80211: Clean up work-queues on disassociation greearb
2013-02-20 6:23 ` Julian Calaby
2013-02-20 14:04 ` Ben Greear
2013-02-20 22:08 ` Julian Calaby
2013-02-20 9:59 ` Johannes Berg
2013-02-20 14:09 ` Ben Greear
2013-02-20 14:13 ` Johannes Berg
2013-02-20 14:24 ` Ben Greear
2013-02-20 14:27 ` Johannes Berg
2013-02-25 10:20 ` Stanislaw Gruszka [this message]
2013-02-25 16:55 ` Ben Greear
2013-02-26 15:55 ` Stanislaw Gruszka
2013-02-26 16:51 ` Ben Greear
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130225102020.GA1735@redhat.com \
--to=sgruszka@redhat.com \
--cc=greearb@candelatech.com \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).