Re: Playing with SATA NCQ

[Jens Axboe]

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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/