From: wgong@codeaurora.org
To: Justin Capella <justincapella@gmail.com>
Cc: linux-wireless@vger.kernel.org, ath10k <ath10k@lists.infradead.org>
Subject: Re: [PATCH v2 1/2] ath10k: add refcount for ath10k_core_restart
Date: Wed, 01 Jan 2020 20:46:40 -0800 [thread overview]
Message-ID: <37eab887aaa350be0489f7e19f17d024@codeaurora.org> (raw)
In-Reply-To: <CAMrEMU_eLfD6=7esrYJA4GnU=CcWgBey1QZL+-=RVoRUUcfZKw@mail.gmail.com>
On 2020-01-01 19:10, Justin Capella wrote:
> Instead of the atomic restart count, can the state be updated to
> ATH10K_STATE_RESTARTING while holding
> mutex_unlock(&ar->conf_mutex);
>
the recovery process is begin with ath10k_core_restart, and end with
ath10k_reconfig_complete.
I already see it has mutex_lock(&ar->conf_mutex) and
mutex_unlock(&ar->conf_mutex) in ath10k_core_restart,
but it is not enough, for example:
1st recovery has finished ath10k_core_restart, but not arrive
ath10k_reconfig_complete, then the 2nd recovery
begin to enter ath10k_core_restart, it will destroy the 1st recovery and
let 1st recovery fail.
After apply this patch, after recovery about 18000+ times, and still can
connect/scan/ping success.
> I don't understand the bundles, but I wonder about the case when there
> are multiple packets (n_rx_pkts) and if pkt_bundle_len might be the
> one to check. Also if there needs to be a check that the len > sizeof
> HTC HDR.
>
the htc_hdr->len is len of payload, so it allow < sizeof HTC HDR, but
not allow > ATH10K_HTC_MBOX_MAX_PAYLOAD_LENGTH.
pkt_bundle is only used when it has many packet in rx side, otherwise it
is not bundled in rx.
patch v3:
https://patchwork.kernel.org/patch/11313853/
https://patchwork.kernel.org/patch/11313859/
> On Tue, Dec 31, 2019 at 1:37 AM <wgong@codeaurora.org> wrote:
>>
>> On 2019-12-25 23:14, Justin Capella wrote:
>> > This does not only effect SDIO.
>> >
>> > Why a semaphore / count? Could the conf_mutex be held earlier, or
>> > perhaps change the state to ATH10K_STATE_RESTARTING first?
>> > ath10k_reconfig_complete is also called in mac.c when channel is
>> > changed so
>> patch v2:
>> https://patchwork.kernel.org/patch/11313853/
>> https://patchwork.kernel.org/patch/11313859/
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
WARNING: multiple messages have this Message-ID (diff)
From: wgong@codeaurora.org
To: Justin Capella <justincapella@gmail.com>
Cc: ath10k <ath10k@lists.infradead.org>, linux-wireless@vger.kernel.org
Subject: Re: [PATCH v2 1/2] ath10k: add refcount for ath10k_core_restart
Date: Wed, 01 Jan 2020 20:46:40 -0800 [thread overview]
Message-ID: <37eab887aaa350be0489f7e19f17d024@codeaurora.org> (raw)
In-Reply-To: <CAMrEMU_eLfD6=7esrYJA4GnU=CcWgBey1QZL+-=RVoRUUcfZKw@mail.gmail.com>
On 2020-01-01 19:10, Justin Capella wrote:
> Instead of the atomic restart count, can the state be updated to
> ATH10K_STATE_RESTARTING while holding
> mutex_unlock(&ar->conf_mutex);
>
the recovery process is begin with ath10k_core_restart, and end with
ath10k_reconfig_complete.
I already see it has mutex_lock(&ar->conf_mutex) and
mutex_unlock(&ar->conf_mutex) in ath10k_core_restart,
but it is not enough, for example:
1st recovery has finished ath10k_core_restart, but not arrive
ath10k_reconfig_complete, then the 2nd recovery
begin to enter ath10k_core_restart, it will destroy the 1st recovery and
let 1st recovery fail.
After apply this patch, after recovery about 18000+ times, and still can
connect/scan/ping success.
> I don't understand the bundles, but I wonder about the case when there
> are multiple packets (n_rx_pkts) and if pkt_bundle_len might be the
> one to check. Also if there needs to be a check that the len > sizeof
> HTC HDR.
>
the htc_hdr->len is len of payload, so it allow < sizeof HTC HDR, but
not allow > ATH10K_HTC_MBOX_MAX_PAYLOAD_LENGTH.
pkt_bundle is only used when it has many packet in rx side, otherwise it
is not bundled in rx.
patch v3:
https://patchwork.kernel.org/patch/11313853/
https://patchwork.kernel.org/patch/11313859/
> On Tue, Dec 31, 2019 at 1:37 AM <wgong@codeaurora.org> wrote:
>>
>> On 2019-12-25 23:14, Justin Capella wrote:
>> > This does not only effect SDIO.
>> >
>> > Why a semaphore / count? Could the conf_mutex be held earlier, or
>> > perhaps change the state to ATH10K_STATE_RESTARTING first?
>> > ath10k_reconfig_complete is also called in mac.c when channel is
>> > changed so
>> patch v2:
>> https://patchwork.kernel.org/patch/11313853/
>> https://patchwork.kernel.org/patch/11313859/
next prev parent reply other threads:[~2020-01-02 4:46 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-25 12:00 [PATCH v2 0/2] start recovery process when payload length overflow for sdio Wen Gong
2019-12-25 12:00 ` Wen Gong
2019-12-25 12:00 ` [PATCH v2 1/2] ath10k: add refcount for ath10k_core_restart Wen Gong
2019-12-25 12:00 ` Wen Gong
2019-12-25 15:14 ` Justin Capella
2019-12-25 15:14 ` Justin Capella
2019-12-31 9:37 ` wgong
2019-12-31 9:37 ` wgong
2020-01-02 3:10 ` Justin Capella
2020-01-02 3:10 ` Justin Capella
2020-01-02 4:46 ` wgong [this message]
2020-01-02 4:46 ` wgong
2019-12-25 12:00 ` [PATCH v2 2/2] ath10k: start recovery process when payload length exceeds max htc length for sdio Wen Gong
2019-12-25 12:00 ` Wen Gong
2019-12-25 22:56 ` Justin Capella
2019-12-25 22:56 ` Justin Capella
2020-01-02 4:50 ` wgong
2020-01-02 4:50 ` wgong
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=37eab887aaa350be0489f7e19f17d024@codeaurora.org \
--to=wgong@codeaurora.org \
--cc=ath10k@lists.infradead.org \
--cc=justincapella@gmail.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 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.