[RFT] net80211 TX serialisation, take #4

[PseudoCylon]

> ------------------------------
>
> Message: 12
> Date: Fri, 22 Feb 2013 17:18:44 -0800
> From: Adrian Chadd <adrian at freebsd.org>
> To: freebsd-wireless at freebsd.org
> Subject: Re: [RFT] net80211 TX serialisation, take #4
> Message-ID:
>         <CAJ-Vmom8HNLNc-=CogiF1v2pJHcn73rB0w9EOHoBtTjAp=jReA at mail.gmail.com>
> Content-Type: text/plain; charset=ISO-8859-1
>
> On 22 February 2013 15:25, Adrian Chadd <adrian at freebsd.org> wrote:
>
> So, this is all pretty terrible. The only sane solution for now is to
> make my VAP TX lock an IC TX lock,and grab said IC TX lock for all
> VAPs. That way the driver can grab the IC TX lock when it's doing
> deferred sends and it'll be sure the lock is held when it decides to
> grab/increment sequence numbers from ni->ni_txseqs[].

I don't think
  lock();
  ni->ni_txseqs[]++;
  unlock();
can fix the problem because no one can guarantee which process/thread
grabs the lock next.

i.e. concurrently, without lock,
- Thread A is processing packet seq#1
- Thread B is processing packet seq#2
When the time to do ni_txseqs[]++, how can we guarantee thread A grabs
the lock before thread B? In other word, if thread A always grabs the
lock first, we don't need to serialize the Tx, do we?

> * .. and do this without tearing my hair out.

The sequence will be messed up during moving packets from one queue to
another, i.e from driver queue to hardware queue. As long as packets
are in a queue (in a linked list) sequence won't change unless we
explicitly write such code. So...

Saving your hair option 1
tx()
{
        for() {
                lock();
                dequeue(m);    /* assuming queue is in order */
                ni_txseqs[]++
                enqueue_working_queue(m);
                unlock();
                ...
                process m
                ...
                lock();
                /*
                 * m may change here.
                 * Whichever the thread does dequeues, m0 will be
                 * the head of the queue, so sequence will be kept intact.
                 * But, need to test if processing of m0 has been completed.
                 */
                dequeue_working_queue(m0);
                enqueue_driver_queue(m0);  /* or hardware_queue() */
                unlock();
        }
}
This will keep sequence intact.

Saving your hair option 2
Shave your head until you fix the problem. Hair will grow again, you
know. No bad hair day is a bonus.


AK