On Wed, Oct 22 2008, Elias Oltmanns wrote:
> Jens Axboe <jens.axboe@xxxxxxxxxx> wrote:
> > We very rarely (if ever) complete more than one command in the
> > sactive mask at the time, even for extremely high IO rates. So
> > looping over the entire range of possible tags is pointless,
> > instead use __ffs() to just find the completed tags directly.
> >
> > Signed-off-by: Jens Axboe <jens.axboe@xxxxxxxxxx>
> > ---
> > drivers/ata/libata-core.c | 15 +++++++++------
> > 1 files changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> > index 1ee9499..c3c53e7 100644
> > --- a/drivers/ata/libata-core.c
> > +++ b/drivers/ata/libata-core.c
> > @@ -4799,9 +4799,9 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
> > */
> > int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active)
> > {
> > + unsigned int i = 0;
> > int nr_done = 0;
> > u32 done_mask;
> > - int i;
> >
> > done_mask = ap->qc_active ^ qc_active;
> >
> > @@ -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;
>
> If you think about it, this statement is equivalent to
>
> if (ap->qc_active ^ qc_active == (1 << (ATA_MAX_QUEUE - 1)))
>
> To fix this, you could say
>
> if (++next + i >= ATA_MAX_QUEUE)
>
> but perhaps it would be even more efficient (or not much worse) to skip
> this check entirely.
Yeah, the check should just be killed, that's the version I posted in
the reply to Tejun as well.
--
Jens Axboe
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
