Re: Playing with SATA NCQ

Jens Axboe
2005-05-27T07:38:42Z

On Fri, May 27 2005, Jeff Garzik wrote:
> Jens Axboe wrote:
> >I double checked this. If you agree to move the setting of QCFLAG_ACTIVE
> >_after_ successful ap->ops->qc_issue(qc) and clear it _after_
> >__ata_qc_complete(qc) then I can just use that bit and kill
> >ATA_QCFLAG_ACCOUNT.
> >
> >What do you think?
>
> Fine with me.
>
> Keep in mind that the attached patch was applied recently...

Yeah, the two hunks from the ncq patch would look like this then. Ok?

Index: drivers/scsi/libata-core.c
===================================================================
--- 3ac9a34948049bff79a2b2ce49c0a3c84e35a748/drivers/scsi/libata-core.c (mode:100644)
+++ uncommitted/drivers/scsi/libata-core.c (mode:100644)
@@ -2812,15 +2893,15 @@

/* call completion callback */
rc = qc->complete_fn(qc, drv_stat);
- qc->flags &= ~ATA_QCFLAG_ACTIVE;

/* if callback indicates not to complete command (non-zero),
* return immediately
*/
- if (rc != 0)
+ if (unlikely(rc != 0))
return;

__ata_qc_complete(qc);
+ qc->flags &= ~ATA_QCFLAG_ACTIVE;

VPRINTK("EXIT\n");
}
@@ -2884,12 +3026,25 @@
ap->ops->qc_prep(qc);

qc->ap->active_tag = qc->tag;
+
+ rc = ap->ops->qc_issue(qc);
+ if (rc != ATA_QC_ISSUE_OK)
+ goto err_out;
+
qc->flags |= ATA_QCFLAG_ACTIVE;

- return ap->ops->qc_issue(qc);
+ if (qc->flags & ATA_QCFLAG_NCQ) {
+ assert(ap->ncq_depth < ATA_MAX_QUEUE)
+ ap->ncq_depth++;
+ } else {
+ assert(!ap->depth);
+ ap->depth++;
+ }

+ return ATA_QC_ISSUE_OK;
err_out:
- return -1;
+ ata_qc_free(qc);
+ return rc;
}

/**

--
Jens Axboe