All of lore.kernel.org
 help / color / mirror / Atom feed
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:30:14 -0800	[thread overview]
Message-ID: <50CA1EB6.9030903@candelatech.com> (raw)
In-Reply-To: <1355423047.9463.9.camel@jlt4.sipsolutions.net>

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?

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


  parent reply	other threads:[~2012-12-13 18:30 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 [this message]
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
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=50CA1EB6.9030903@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.