From: Johannes Berg <johannes@sipsolutions.net>
To: Ben Greear <greearb@candelatech.com>
Cc: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: Re: Add a new work-queue for destructing stations?
Date: Thu, 13 Dec 2012 19:49:57 +0100 [thread overview]
Message-ID: <1355424597.9463.17.camel@jlt4.sipsolutions.net> (raw)
In-Reply-To: <50CA22BC.7090008@candelatech.com>
On Thu, 2012-12-13 at 10:47 -0800, Ben Greear wrote:
> On 12/13/2012 10:41 AM, Johannes Berg wrote:
> > On Thu, 2012-12-13 at 10:39 -0800, Ben Greear wrote:
> >
> >>>> So, near where we currently call cancel_work_sync(&sdata->work), I
> >>>> think we'd also need to cancel things like the sta->ampdu_mlme.work,
> >>>> and probably others as well.
> >>>>
> >>>> Then, when we're sure there are no more references to sdata on the
> >>>> main-work-queue, it would be safe to flush the destructor-workqueue
> >>>> (which would have the fre_sta_work() on it).
> >>>>
> >>>> Maybe?
> >>>
> >>> Huh, ok, but I don't think you'll find much that doesn't reference an
> >>> sdata?
> >>
> >> I think you are right...so just need to find all of those work items
> >> and call cancel_work_sync() on them, just like we are currently cancelling
> >> the sdata->work....
> >
> > Well the concern there is that we expose the workqueue to the driver,
> > and it might even put sdata-referencing work items on it and expect them
> > to be flushed out before stop_interface() is called?
> >
> > You'd be welcome to change the API so that isn't true, or things are
> > different, or whatever, but it'd be a lot of careful auditing (and
> > probably fixing) of drivers.
>
> So, maybe a new driver api call to 'cancel_all_work_items(sdata)',
> and if the driver does not implement this, then we stick with the
> full flush_workqueue() in mac80211_do_stop(). But, if the driver
> does implement it, and we add code there to flush any mac80211 related
> work-items, then we could skip the flush_workqueue() call....
You don't even need the API, it can do it in remove_interface(). It's
just not ... guaranteed right now. Also, if it wants them to run,
locking may get tricky?
johannes
next prev parent reply other threads:[~2012-12-13 18:49 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-13 17:46 Add a new work-queue for destructing stations? Ben Greear
2012-12-13 17:59 ` Johannes Berg
2012-12-13 18:19 ` Ben Greear
2012-12-13 18:24 ` Johannes Berg
2012-12-13 18:27 ` Johannes Berg
2012-12-13 18:36 ` Johannes Berg
2012-12-13 18:30 ` Ben Greear
2012-12-13 18:37 ` Johannes Berg
2012-12-13 18:39 ` Ben Greear
2012-12-13 18:41 ` Johannes Berg
2012-12-13 18:47 ` Ben Greear
2012-12-13 18:49 ` Johannes Berg [this message]
2012-12-13 18:53 ` Johannes Berg
2012-12-13 19:00 ` Ben Greear
2012-12-13 19:11 ` Johannes Berg
2012-12-13 19:17 ` 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=1355424597.9463.17.camel@jlt4.sipsolutions.net \
--to=johannes@sipsolutions.net \
--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).