On Thu, Oct 23 2008, Jens Axboe wrote:

On Thu, Oct 23 2008, Tejun Heo wrote:

quoted

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;
}

That doesn't work (you're adding next to tag twice), it needs a little
tweak:

while (done_mask) {
        struct ata_queued_cmd *qc;
        unsigned int next = __ffs(done_mask);

        if ((qc = ata_qc_from_tag(ap, tag + next))) {
                ata_qc_complete(qc);
                nr_done++;
        }
        next++;
        tag += next;
        done_mask >>= next;
}

and I think it should work. Not tested yet :-)

Pondered some more, and it can't work. The problem is that if we
complete tag 31, we attempt to shift done_mask down by 32 bits. On a
32-bit arch, that's not defined. So we DO need a check like the existing
one, or something similar.

So I don't think we need to make changes to this patch either, at least
unless one of you can come up with a better check that avoids a branch.

-- 
Jens Axboe