Messages in this thread | | | Date | Thu, 23 Oct 2008 08:37:16 +0200 | From | Jens Axboe <> | Subject | Re: [PATCH 1/2] libata: get rid of ATA_MAX_QUEUE loop in ata_qc_complete_multiple() |
| |
On Thu, Oct 23 2008, Tejun Heo wrote: > Jens Axboe wrote: > > @@ -4811,16 +4811,19 @@ int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active) > > return -EINVAL; > > } > > > > - for (i = 0; i < ATA_MAX_QUEUE; i++) { > > + while (done_mask) { > > struct ata_queued_cmd *qc; > > + unsigned int next = __ffs(done_mask); > > > > - if (!(done_mask & (1 << i))) > > - continue; > > - > > - if ((qc = ata_qc_from_tag(ap, i))) { > > + qc = ata_qc_from_tag(ap, i + next); > > + if (qc) { > > ata_qc_complete(qc); > > nr_done++; > > } > > + if (++next >= ATA_MAX_QUEUE) > > + break; > > + i += next; > > + done_mask >>= next; > > Shouldn't this be... > > i += next + 1; > if (i >= ATA_MAX_QUEUE) > break; > > or better... > > while (done_mask) { > struct ata_queued_cmd *qc; > unsigned int next = __ffs(done_mask); > > tag += next; > if ((qc = ata_qc_from_tag(ap, tag))) { > ata_qc_complete(qc); > nr_done++; > } > next++; > tag += next; > done_mask >>= next; > } > > done_mask is guaranteed to be zero at when tag reaches ATA_MAX_QUEUE.
That does indeed look a lot cleaner. I think it was rewritten at some point and kept some of the logic for not passing 0 into __ffs, but it's clearly pointless now.
I'll send out a revised patch when it's tested.
-- Jens Axboe
| |