From: Ben Greear <greearb@candelatech.com>
To: Johannes Berg <johannes@sipsolutions.net>
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 10:39:16 -0800 [thread overview]
Message-ID: <50CA20D4.90707@candelatech.com> (raw)
In-Reply-To: <1355423836.9463.14.camel@jlt4.sipsolutions.net>
On 12/13/2012 10:37 AM, Johannes Berg wrote:
> On Thu, 2012-12-13 at 10:30 -0800, Ben Greear wrote:
>> On 12/13/2012 10:24 AM, Johannes Berg wrote:
>>> On Thu, 2012-12-13 at 10:19 -0800, Ben Greear wrote:
>>>
>>>>> I don't think that's easy, but you're welcome to try. The
>>>>> free_sta_work() function references the sdata so it absolutely must run
>>>>> at this point.
>>>
>>>> So, cancel_work_sync(&sdata->work) would appear to remove
>>>> all pending sdata->work items from the work-queue. As long as
>>>> there are no other different work items that reference
>>>> sdata (and maybe there are..I haven't looked at all of them),
>>>> then we should be safe to execute the free_sta_work()
>>>> on a different work-queue safely, I think....
>>>
>>> Sorry, I don't get it. free_sta_work() *itself* has to be executed
>>> before the sdata is destroyed. cancel_work_sync(&sdata->work) has
>>> nothing to do with free_sta_work.
>>
>> My concern is this:
>>
>> If I add a different work queue (called destructor-workqueue, perhaps)
>> for free_sta_work(), then it only helps
>> if I can not block on flushing the current 'big' work-queue. But, if
>> there are items on the big work-queue that reference sdata, then
>> that could easily execute after free_sta_work(), and I'm guessing
>> that would be very bad.
>>
>> 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....
Thanks,
Ben
>
> johannes
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
next prev parent reply other threads:[~2012-12-13 18:39 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 [this message]
2012-12-13 18:41 ` Johannes Berg
2012-12-13 18:47 ` Ben Greear
2012-12-13 18:49 ` Johannes Berg
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=50CA20D4.90707@candelatech.com \
--to=greearb@candelatech.com \
--cc=johannes@sipsolutions.net \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.