Re: [PATCH] wmi: Retry if CE logic is out of buffers.

From: Michal Kazior <michal.kazior@tieto.com>
To: Ben Greear <greearb@candelatech.com>
Cc: "ath10k@lists.infradead.org" <ath10k@lists.infradead.org>
Subject: Re: [PATCH] wmi: Retry if CE logic is out of buffers.
Date: Thu, 31 Mar 2016 08:55:29 +0200	[thread overview]
Message-ID: <CA+BoTQm5-Yarw9TTLiN4jvuH2cdELxRBQvJ_+TFmthZkMBOVyg@mail.gmail.com> (raw)
In-Reply-To: <1459368643-13300-1-git-send-email-greearb@candelatech.com>

On 30 March 2016 at 22:10,  <greearb@candelatech.com> wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> I believe the CE tx buffer reaping logic may be able to fall
> behind in certain cases (lots of serial console logging, lots
> of WMI messages).
>
> Dropping WMI messages is a very serious problem, so it is worth
> waiting a bit in hopes the tx buffers become available again.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>
> Probably the ath10k_err should be made dbg or rate-limited before
> this goes upstream..in meantime, it might help shed some light on
> this problem.
>
>  drivers/net/wireless/ath/ath10k/wmi.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> index f042711..43d23fc 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -1819,6 +1819,7 @@ static void ath10k_wmi_op_ep_tx_credits(struct ath10k *ar)
>  int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb, u32 cmd_id)
>  {
>         int ret = -EOPNOTSUPP;
> +       int retry = 1000;
>
>         might_sleep();
>
> @@ -1832,7 +1833,19 @@ int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb, u32 cmd_id)
>                 /* try to send pending beacons first. they take priority */
>                 ath10k_wmi_tx_beacons_nowait(ar);
>
> -               ret = ath10k_wmi_cmd_send_nowait(ar, skb, cmd_id);
> +               while (--retry) {
> +                       ret = ath10k_wmi_cmd_send_nowait(ar, skb, cmd_id);
> +                       if ((ret == -ENOBUFS) &&
> +                           !test_bit(ATH10K_FLAG_CRASH_FLUSH, &ar->dev_flags)) {
> +                               /* CE transport logic is full, maybe we cannot reap entries fast
> +                                * enough?
> +                                */
> +                               ath10k_err(ar, "CE transport is full, sleeping for 1ms\n");
> +                               msleep(1);
> +                               continue;
> +                       }
> +                       break;
> +               }

This looks like a workaround to me. This problem shouldn't be
happening in the first place as far as design is concerned.

If it does the only reason I can think of is if MSI-range support is exercised.

Anyway, It'd be a lot more sane to instead poll WMI's CE Tx pipe when
processing WMI's CE Rx pipe (but that's still -arguably- unnecessary)
instead of retrying in WMI..

Michał

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