[PATCH] libata: implement ata_exec_internal()

This patch implements ata_exec_internal() function which performs
libata internal command execution.  Previously, this was done by each
user by manually initializing a qc, issueing it, waiting for its
completion and handling errors.  In addition to obvious code
factoring, using ata_exec_internal() fixes the following bugs.

* qc not freed on issue failure
* ap->qactive clearing could race with the next internal command
* race between timeout handling and irq
* ignoring error condition not represented in tf->status

Also, qc & hardware are not accessed anymore once it's completed,
making internal commands more conformant with general semantics.
ata_exec_internal() also makes it easy to issue internal commands from
multiple threads if that becomes necessary.

This patch only implements ata_exec_internal().  A following patch
will convert all users.

Signed-off-by: Tejun Heo <htejun@gmail.com>
--

Jeff, all patches have been regenerated against upstream branch as of
today.  (575ab52a218e4ff0667a6cbd972c3af443ee8713)

Also, I took out a debug printk from ata_exec_internal (don't know how
that one got left there).  Other than that, all patches are identical
to the previous posting.

Thanks. :-)
Signed-off-by: Jeff Garzik <jgarzik@pobox.com>

diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index a0060cf31e0d8fd26180afbda64d1342510c2a94..de80abeab065f6f46b626d254d56e5f5d6fa218b 100644 (file)
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1046,6 +1046,105 @@ static unsigned int ata_pio_modes(const struct ata_device *adev)
        return modes;
 }
 
+struct ata_exec_internal_arg {
+       unsigned int err_mask;
+       struct ata_taskfile *tf;
+       struct completion *waiting;
+};
+
+int ata_qc_complete_internal(struct ata_queued_cmd *qc)
+{
+       struct ata_exec_internal_arg *arg = qc->private_data;
+       struct completion *waiting = arg->waiting;
+
+       if (!(qc->err_mask & ~AC_ERR_DEV))
+               qc->ap->ops->tf_read(qc->ap, arg->tf);
+       arg->err_mask = qc->err_mask;
+       arg->waiting = NULL;
+       complete(waiting);
+
+       return 0;
+}
+
+/**
+ *     ata_exec_internal - execute libata internal command
+ *     @ap: Port to which the command is sent
+ *     @dev: Device to which the command is sent
+ *     @tf: Taskfile registers for the command and the result
+ *     @dma_dir: Data tranfer direction of the command
+ *     @buf: Data buffer of the command
+ *     @buflen: Length of data buffer
+ *
+ *     Executes libata internal command with timeout.  @tf contains
+ *     command on entry and result on return.  Timeout and error
+ *     conditions are reported via return value.  No recovery action
+ *     is taken after a command times out.  It's caller's duty to
+ *     clean up after timeout.
+ *
+ *     LOCKING:
+ *     None.  Should be called with kernel context, might sleep.
+ */
+
+static unsigned
+ata_exec_internal(struct ata_port *ap, struct ata_device *dev,
+                 struct ata_taskfile *tf,
+                 int dma_dir, void *buf, unsigned int buflen)
+{
+       u8 command = tf->command;
+       struct ata_queued_cmd *qc;
+       DECLARE_COMPLETION(wait);
+       unsigned long flags;
+       struct ata_exec_internal_arg arg;
+
+       spin_lock_irqsave(&ap->host_set->lock, flags);
+
+       qc = ata_qc_new_init(ap, dev);
+       BUG_ON(qc == NULL);
+
+       qc->tf = *tf;
+       qc->dma_dir = dma_dir;
+       if (dma_dir != DMA_NONE) {
+               ata_sg_init_one(qc, buf, buflen);
+               qc->nsect = buflen / ATA_SECT_SIZE;
+       }
+
+       arg.waiting = &wait;
+       arg.tf = tf;
+       qc->private_data = &arg;
+       qc->complete_fn = ata_qc_complete_internal;
+
+       if (ata_qc_issue(qc))
+               goto issue_fail;
+
+       spin_unlock_irqrestore(&ap->host_set->lock, flags);
+
+       if (!wait_for_completion_timeout(&wait, ATA_TMOUT_INTERNAL)) {
+               spin_lock_irqsave(&ap->host_set->lock, flags);
+
+               /* We're racing with irq here.  If we lose, the
+                * following test prevents us from completing the qc
+                * again.  If completion irq occurs after here but
+                * before the caller cleans up, it will result in a
+                * spurious interrupt.  We can live with that.
+                */
+               if (arg.waiting) {
+                       qc->err_mask = AC_ERR_OTHER;
+                       ata_qc_complete(qc);
+                       printk(KERN_WARNING "ata%u: qc timeout (cmd 0x%x)\n",
+                              ap->id, command);
+               }
+
+               spin_unlock_irqrestore(&ap->host_set->lock, flags);
+       }
+
+       return arg.err_mask;
+
+ issue_fail:
+       ata_qc_free(qc);
+       spin_unlock_irqrestore(&ap->host_set->lock, flags);
+       return AC_ERR_OTHER;
+}
+
 static int ata_qc_wait_err(struct ata_queued_cmd *qc,
                           struct completion *wait)
 {

diff --git a/include/linux/libata.h b/include/linux/libata.h
index e18ce039cdfd53614c5663abb5be8bb5210a9705..833e57afd54c3b23025bd2216cac7eff01e982c8 100644 (file)
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -135,6 +135,8 @@ enum {
        ATA_TMOUT_BOOT_QUICK    = 7 * HZ,       /* hueristic */
        ATA_TMOUT_CDB           = 30 * HZ,
        ATA_TMOUT_CDB_QUICK     = 5 * HZ,
+       ATA_TMOUT_INTERNAL      = 30 * HZ,
+       ATA_TMOUT_INTERNAL_QUICK = 5 * HZ,
 
        /* ATA bus states */
        BUS_UNKNOWN             = 0,