[rt2x00-users] [PATCH 4/4] rt2x00: Implement TX status reporting for rt2800usb

Ivo Van Doorn ivdoorn at gmail.com
Sat Jul 31 21:40:03 AEST 2010


On Sat, Jul 31, 2010 at 1:04 PM, Helmut Schaa
<helmut.schaa at googlemail.com> wrote:
> Hi Ivo,
>
> Am Samstag 31 Juli 2010 schrieb Ivo van Doorn:
>> The TX_STA_FIFO register which is used for per-frame TX frame
>> status reporting is also valid on rt2800usb. We can move the
>> rt2800pci_txdone function into rt2800lib where it can also
>> be used by rt2800usb.
>>
>> rt2800usb needs to overwrite the txdone work handler to
>> a different function.
>>
>> Both rt2800usb as rt2800_txdone need to take into account
>> that IO failures can occur while uploading the URB, which
>> means that when obtaining the new entry the IO status must
>> be checked.
>>
>> Signed-off-by: Ivo van Doorn <IvDoorn at gmail.com>
>
> [...]
>
>> +static void rt2800usb_work_txdone(struct work_struct *work)
>> +{
>> +     struct rt2x00_dev *rt2x00dev =
>> +         container_of(work, struct rt2x00_dev, txdone_work);
>> +     struct data_queue *queue;
>> +     struct queue_entry *entry;
>> +
>> +     rt2800_txdone(rt2x00dev);
>> +
>> +     tx_queue_for_each(rt2x00dev, queue) {
>> +             while (!rt2x00queue_empty(queue)) {
>> +                     entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
>> +
>> +                     if (test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags) ||
>> +                         !test_bit(ENTRY_DATA_IO_FAILED, &entry->flags))
>> +                             break;
>> +
>> +                     rt2x00lib_txdone_noinfo(entry, TXDONE_FAILURE);
>> +             }
>> +     }
>> +}
>
> Wouldn't that mess up the tx status reporting in some cases? Assume the
> following situation:
>
> Frames 1-3 are submitted to the hw, frames 1 and 2 are sent out immediately
> but frame 3 has to wait due to a high contention on the air for example. So,
> when rt2800usb_work_txdone is run the TX_STA_FIFO contains two entries for
> frames 1 and 2 but none for frame 3. The tx status for 1 and 2 is reported
> correctly to mac80211 whereas frame 3 is reported with a default status (which
> is ok). However. once frame 3 is really sent out the hw will push its real
> tx status into TX_STA_FIFO. Sending frame 4 now will match the tx status of
> frame 3 to frame 4 whereas the tx status of frame 4 will be kept in the FIFO
> till the next frame is sent, resulting in an incorrect tx status <-> frame
> association.

This situation wouldn't occur. If 3 frames are submitted to the hardware,
each of them reports back if the upload succeeded. If that is the case
the flag ENTRY_OWNER_DEVICE_DATA is cleared. If the URB failed,
then ENTRY_DATA_IO_FAILED is set, otherwise the driver will wait for
the TX_STA_FIFO report.

Lets assume your scenario, 3 frames were uploaded succesfully,
but TX_STA_FIFO contains 2 entries. Then only the status of 2 entries
are reported to mac80211. After the call to rt2800_txdone() rt2800usb,
needs to check if any URB has come back with a failed report (which
means that ENTRY_DATA_IO_FAILED is set), if that is not the case
(in your example frame 3), then nothing will happen until the next
frame is sucessfully uploaded and the rt2800usb_work_txdone
handler runs again.

The handler works in such a way that when it ends it has moved the
INDEX_DONE pointer to the last entry which was has either not
completed the upload yet, or to the entry for which no status was
reported yet.

So the mismatch of frames will only occur, when the TX_STA_FIFO
is not filled in for all frames (which is occuring in rt61pci), but then we
have the same problem as rt61pci and rt2800pci. However you reported
that this problem only occurs when the TX_STA_FIFO overflows, but
that is not happening in the rt2800usb case, since the interrupt handler
is running much more often. Which reduces the chance of overflowing.

> As this still might improve rt2800usb performance it's still far from
> a reliable tx status reporting. At least on rt2800pci when the tx status
> reports don't match the real frames the rate control algortihms often end up
> using a lower rate then possible.
>
> Don't get me wrong, I don't have any objections against this patch series
> to get merged (as I guess you did some tests and noticed better performance
> with rt2800usb, right ;) ? ) but wanted to point out a few things I've noticed
> while working on the tx status reporting in rt2800pci.

I didn't notice better performance, as with the old situation rt2800usb always
reports success. Thus the rate control never switched down when the performance
decreased. I can now walk away from my AP, and the rate is lowered accordingly.

> But at least this makes rt2800usb and rt2800pci to suffer from the same issue
> and we might be able to fix it for both drivers in one shot ;)

Exactly :)

> I didn't review the whole series yet but I might find some time to do so soon,

Thanks,

Ivo




More information about the users mailing list