linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: Eyal Reizer <eyalr@ti.com>, Guy Mishol <guym@ti.com>,
	linux-wireless@vger.kernel.org, linux-omap@vger.kernel.org
Subject: Re: [PATCH 1/4] wlcore: Use spin_trylock in wlcore_irq_locked() for running the queue
Date: Mon, 22 Jun 2020 09:06:28 -0700	[thread overview]
Message-ID: <20200622160628.GL37466@atomide.com> (raw)
In-Reply-To: <875zbjgpbj.fsf@codeaurora.org>

* Kalle Valo <kvalo@codeaurora.org> [200622 14:15]:
> Tony Lindgren <tony@atomide.com> writes:
> 
> > We need the spinlock to check if we need to run the queue. Let's use
> > spin_trylock instead and always run the queue unless we know there's
> > nothing to do.
> 
> Why? What's the problem you are solving here?

To simplify the flags and locking use between the threaded irq
and tx work.

While chasing an occasional hang with an idle wlan doing just a
periodic network scans, I noticed we can start simplifying the
locking between the threaded irq and tx work for the driver.

No luck so far figuring out what the occasional idle wlan hang is,
but I suspect we end up somewhere in a deadlock between tx work
and the threaded irq.

We currently have a collection of flags and locking between the
threaded irq and tx work:

- wl->flags bitops
- wl->mutex
- wl->wl_lock spinlock

The bitops flags do not need a spinlock around them, and
wlcore_irq() already holds the mutex calling wlcore_irq_locked().
And we only need the spinlock to see if we need to run the queue
or not.

So I think eventually we can remove most of the spinlock use in
favor of the mutex. I guess I could leave out the trylock changes
here if this is too many changes at once.

Or do you see some problem in general with this approach?

Regards,

Tony

  reply	other threads:[~2020-06-22 16:06 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-17 21:25 [PATCH 0/4] Improvments for wlcore irq and resume for v5.9 Tony Lindgren
2020-06-17 21:25 ` [PATCH 1/4] wlcore: Use spin_trylock in wlcore_irq_locked() for running the queue Tony Lindgren
2020-06-22 14:14   ` Kalle Valo
2020-06-22 16:06     ` Tony Lindgren [this message]
2020-06-23  6:41       ` Kalle Valo
2020-06-23 18:48         ` Tony Lindgren
2020-06-17 21:25 ` [PATCH 2/4] wlcore: Use spin_trylock in wlcore_irq() to see if we need to queue tx Tony Lindgren
2020-06-22 14:16   ` Kalle Valo
2020-06-17 21:25 ` [PATCH 3/4] wlcore: Simplify runtime resume ELP path Tony Lindgren
2020-06-17 21:25 ` [PATCH 4/4] wlcore: Remove pointless spinlock Tony Lindgren

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=20200622160628.GL37466@atomide.com \
    --to=tony@atomide.com \
    --cc=eyalr@ti.com \
    --cc=guym@ti.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-omap@vger.kernel.org \
    --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).