All of lore.kernel.org
 help / color / mirror / Atom feed
From: Larry Finger <Larry.Finger@lwfinger.net>
To: Christian Lamparter <chunkeey@web.de>
Cc: linux-wireless <linux-wireless@vger.kernel.org>,
	Johannes Berg <johannes@sipsolutions.net>
Subject: Re: [WIP] p54: deal with allocation failures in rx path
Date: Sat, 04 Jul 2009 16:14:02 -0500	[thread overview]
Message-ID: <4A4FC61A.30004@lwfinger.net> (raw)
In-Reply-To: <4A4FB3F2.5050405@lwfinger.net>

Larry Finger wrote:
> @@ -224,6 +236,7 @@ static void p54_tx_qos_accounting_free(s
>                 struct p54_tx_data *data = (void *) hdr->data;
> 
>                 priv->tx_stats[data->hw_queue].len--;
> +               WARN_ON(priv->tx_stats[data->hw_queue].len < 0);
>         }
>         p54_wake_queues(priv);
>  }
> 

The new WARN_ON did _NOT_ trigger when the len went negative.

The only other place where len could be decremented is through
txhdr->backlog. I noticed that the p54common.c had

txhdr->backlog = current_queue->len;

This was replaced in txrx.c by

txhdr->backlog = priv->tx_stats[queue].len - 1;

Was this intentional?

To test if this is the problem, I added the following hunk:

@@ -840,6 +853,7 @@ int p54_tx_80211(struct ieee80211_hw *de
        txhdr->crypt_offset = crypt_offset;
        txhdr->hw_queue = queue;
        txhdr->backlog = priv->tx_stats[queue].len - 1;
+       WARN_ON(!priv->tx_stats[queue].len);
        memset(txhdr->durations, 0, sizeof(txhdr->durations));
        txhdr->tx_antenna = ((info->antenna_sel_tx == 0) ?
                2 : info->antenna_sel_tx - 1) & priv->tx_diversity_mask;

This WARN_ON did trigger just before txq[6].len went negative. I'm now
testing with that changed as follows:

@@ -839,7 +852,8 @@ int p54_tx_80211(struct ieee80211_hw *de
        }
        txhdr->crypt_offset = crypt_offset;
        txhdr->hw_queue = queue;
-       txhdr->backlog = priv->tx_stats[queue].len - 1;
+       txhdr->backlog = priv->tx_stats[queue].len;
+       WARN_ON(priv->tx_stats[queue].len < 0);
        memset(txhdr->durations, 0, sizeof(txhdr->durations));
        txhdr->tx_antenna = ((info->antenna_sel_tx == 0) ?
                2 : info->antenna_sel_tx - 1) & priv->tx_diversity_mask;

This WARN_ON did not trigger, but I still had the queue len go negative.

One other question: struct p54_burst is defined in lmac.h, but it
doesn't seem to be used anywhere. Will it be needed later?

Larry


  reply	other threads:[~2009-07-04 21:13 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-03 22:53 [WIP] p54: deal with allocation failures in rx path Christian Lamparter
2009-07-04  1:09 ` Larry Finger
2009-07-04  2:16 ` Larry Finger
2009-07-04 10:11   ` Christian Lamparter
2009-07-04 16:40     ` Larry Finger
2009-07-04 17:28       ` Christian Lamparter
2009-07-04 19:56         ` Larry Finger
2009-07-04 21:14           ` Larry Finger [this message]
2009-07-05 13:59             ` Christian Lamparter
2009-07-05 17:49               ` Larry Finger
2009-07-05 22:05                 ` Christian Lamparter
2009-07-06  1:36                   ` Larry Finger
2009-07-06 13:16                     ` Christian Lamparter
2009-07-04  7:52 ` Johannes Berg
2009-07-05  0:56 ` Max Filippov
2009-07-05 14:00   ` Christian Lamparter
2009-07-05 19:16     ` Max Filippov
2009-07-05 22:46       ` Max Filippov
2009-07-06 13:11 ` Max Filippov
2009-07-06 14:00   ` Christian Lamparter
2009-07-06 14:18     ` Max Filippov

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=4A4FC61A.30004@lwfinger.net \
    --to=larry.finger@lwfinger.net \
    --cc=chunkeey@web.de \
    --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.