ath10k.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: linux-wireless <linux-wireless@vger.kernel.org>,
	ath10k <ath10k@lists.infradead.org>,
	Miaoqing Pan <miaoqing@codeaurora.org>
Subject: Re: [PATCH] ath10k: fix wmi mgmt tx queue full due to race condition
Date: Mon, 21 Dec 2020 12:20:09 -0800	[thread overview]
Message-ID: <CA+ASDXMK+8aOvtp-QgkXYXw=BZcFdo9H3ZFSXDsee0PFzUO97Q@mail.gmail.com> (raw)
In-Reply-To: <87ft3zndfr.fsf@codeaurora.org>

On Mon, Dec 21, 2020 at 11:53 AM Kalle Valo <kvalo@codeaurora.org> wrote:
> Brian Norris <briannorris@chromium.org> writes:
> > On Sun, Dec 20, 2020 at 5:53 PM Miaoqing Pan <miaoqing@codeaurora.org> wrote:
> >>         if (skb_queue_len(q) == ATH10K_MAX_NUM_MGMT_PENDING) {
> >
> > I believe you should be switching this to use skb_queue_len_lockless()
> > too.
>
> Why lockless? (reads documentation) Ah, is it due to memory
> synchronisation now that we don't take the data_lock anymore?

As the original post notes, there was no valid locking in the first
place anyway, but now that we're fully relying on the queue lock, we
either need to grab that lock, or else otherwise use lock-free-denoted
methods.

One could say it's about data synchronization, but it's really about
the lack of memory model -- C didn't have a formal one until
relatively recently, and the kernel has always blazed its own way
anyway. You need to annotate *something* about a bare read, otherwise
it's not safe to do concurrently; either compiler or hardware can do
nasty things to you. (In practice, it's unlikely this particular case
will cause a problem for this reason; it's already a somewhat
imprecise check anyway.)

Brian

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

  reply	other threads:[~2020-12-21 20:20 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-21  1:52 [PATCH] ath10k: fix wmi mgmt tx queue full due to race condition Miaoqing Pan
2020-12-21 19:31 ` Brian Norris
2020-12-21 19:53   ` Kalle Valo
2020-12-21 20:20     ` Brian Norris [this message]
2021-01-18 16:16 ` Kalle Valo
     [not found] ` <20210118161650.4B9BAC43461@smtp.codeaurora.org>
2021-01-20  1:33   ` miaoqing

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='CA+ASDXMK+8aOvtp-QgkXYXw=BZcFdo9H3ZFSXDsee0PFzUO97Q@mail.gmail.com' \
    --to=briannorris@chromium.org \
    --cc=ath10k@lists.infradead.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=miaoqing@codeaurora.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).