Hi Ivo
On Sat, Aug 06, 2011 at 01:06:51PM +0200, Ivo Van Doorn wrote:
> On Thu, Aug 4, 2011 at 2:46 PM, Stanislaw Gruszka <sgruszka@xxxxxxxxxx> wrote:
> > -static void rt2800usb_txdone(struct rt2x00_dev *rt2x00dev)
> > +static int rt2800usb_txdone(struct rt2x00_dev *rt2x00dev)
> > {
> > struct data_queue *queue;
> > struct queue_entry *entry;
> > u32 reg;
> > u8 qid;
> >
> > - while (kfifo_get(&rt2x00dev->txstatus_fifo, ®)) {
> > + while (kfifo_peek(&rt2x00dev->txstatus_fifo, ®)) {
>
> I'm not too sure about this change, why do you need to do kfifo_peek
> and add gotos to the end of the while-loop to remove the item from the queue?
> There is no condition in which the obtained value from kfifo-peek
> will require it to be read again later (because when the value couldn't be
> handled we are throwing it away anyway using kfifo_skip).
There is new case (see below) where it is needed. I can get rid of goto,
that will make code a bit cleaner. There is place for optimization, mainly
make tx_status fifo per queue, but for now I just want to fix kernel crashes.
> > /* TX_STA_FIFO_PID_QUEUE is a 2-bit field, thus
> > * qid is guaranteed to be one of the TX QIDs
> > @@ -517,25 +517,39 @@ static void rt2800usb_txdone(struct rt2x00_dev *rt2x00dev)
> > if (unlikely(!queue)) {
> > WARNING(rt2x00dev, "Got TX status for an unavailable "
> > "queue %u, dropping\n", qid);
> > - continue;
> > + goto next_reg;
> > }
> >
> > /*
> > * Inside each queue, we process each entry in a chronological
> > * order. We first check that the queue is not empty.
> > */
> > - entry = NULL;
> > - while (!rt2x00queue_empty(queue)) {
> > + while (1) {
> > + entry = NULL;
> > +
> > + if (rt2x00queue_empty(queue))
> > + break;
> > +
> > entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
> > +
> > + if (test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags) ||
> > + !test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags)) {
> > + WARNING(rt2x00dev, "Data pending for entry %u"
> > + "in queue %u\n", entry->entry_idx, qid);
> > + return 1;
Here is part of code where we exit the loop (and whole function) and do
not remove head "reg" from tx_status fifo - and read it again when
_txdone work is called next time.
Stanislaw
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
