Re: SATA problems

From: Marcus Haebler
Date: Wed Feb 21 2007 - 00:57:38 EST


Tejun,

thanks. In preparation of your patch I installed a vanilla 2.6.20.1
kernel on my FC6
system. Amazingly the problem went away with the vanilla(!) kernel and NCQ
is enabled at boot time (queue_depth is 31). I guess I should have
tried that kernel
earlier.

The patches you sent earlier apply w/o problems against the 2.6.20.1
vanilla kernel
which is expected. I will test drive those patches tomorrow.

BTW thanks for saving me the 'cat' on the 3 patches. ;)

Thanks,

Marcus

On 2/20/07, Tejun Heo <htejun@xxxxxxxxx> wrote:
Marcus Haebler wrote:
> thanks for the patches! I am on an Intel P965/ICH8R.

I see. That can happen too. There was a race window where in-flight
r/w command which left SCSI midlayer but pending on libata gets executed
in the wrong mode. If possible, please verify that it doesn't happen
with the patches applied. I'm attaching combined patch against v2.6.20.

Thanks.

--
tejun

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 667acd2..348cc02 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -308,9 +308,7 @@ int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
tf->flags |= tf_flags;

- if ((dev->flags & (ATA_DFLAG_PIO | ATA_DFLAG_NCQ_OFF |
- ATA_DFLAG_NCQ)) == ATA_DFLAG_NCQ &&
- likely(tag != ATA_TAG_INTERNAL)) {
+ if (ata_ncq_enabled(dev) && likely(tag != ATA_TAG_INTERNAL)) {
/* yay, NCQ */
if (!lba_48_ok(block, n_block))
return -ERANGE;
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 73902d3..ebb9185 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -945,29 +945,32 @@ int ata_scsi_change_queue_depth(struct scsi_device *sdev, int queue_depth)
struct ata_port *ap = ata_shost_to_port(sdev->host);
struct ata_device *dev;
unsigned long flags;
- int max_depth;

- if (queue_depth < 1)
+ if (queue_depth < 1 || queue_depth == sdev->queue_depth)
return sdev->queue_depth;

dev = ata_scsi_find_dev(ap, sdev);
if (!dev || !ata_dev_enabled(dev))
return sdev->queue_depth;

- max_depth = min(sdev->host->can_queue, ata_id_queue_depth(dev->id));
- max_depth = min(ATA_MAX_QUEUE - 1, max_depth);
- if (queue_depth > max_depth)
- queue_depth = max_depth;
-
- scsi_adjust_queue_depth(sdev, MSG_SIMPLE_TAG, queue_depth);
-
+ /* NCQ enabled? */
spin_lock_irqsave(ap->lock, flags);
- if (queue_depth > 1)
- dev->flags &= ~ATA_DFLAG_NCQ_OFF;
- else
+ dev->flags &= ~ATA_DFLAG_NCQ_OFF;
+ if (queue_depth == 1 || !ata_ncq_enabled(dev)) {
dev->flags |= ATA_DFLAG_NCQ_OFF;
+ queue_depth = 1;
+ }
spin_unlock_irqrestore(ap->lock, flags);

+ /* limit and apply queue depth */
+ queue_depth = min(queue_depth, sdev->host->can_queue);
+ queue_depth = min(queue_depth, ata_id_queue_depth(dev->id));
+ queue_depth = min(queue_depth, ATA_MAX_QUEUE - 1);
+
+ if (sdev->queue_depth == queue_depth)
+ return -EINVAL;
+
+ scsi_adjust_queue_depth(sdev, MSG_SIMPLE_TAG, queue_depth);
return queue_depth;
}

@@ -1454,11 +1457,9 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
static int ata_scmd_need_defer(struct ata_device *dev, int is_io)
{
struct ata_port *ap = dev->ap;
+ int is_ncq = is_io && ata_ncq_enabled(dev);

- if (!(dev->flags & ATA_DFLAG_NCQ))
- return 0;
-
- if (is_io) {
+ if (is_ncq) {
if (!ata_tag_valid(ap->active_tag))
return 0;
} else {
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 91bb8ce..4e4e365 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1035,6 +1035,21 @@ static inline u8 ata_chk_status(struct ata_port *ap)
return ap->ops->check_status(ap);
}

+/**
+ * ata_ncq_enabled - Test whether NCQ is enabled
+ * @dev: ATA device to test for
+ *
+ * LOCKING:
+ * spin_lock_irqsave(host lock)
+ *
+ * RETURNS:
+ * 1 if NCQ is enabled for @dev, 0 otherwise.
+ */
+static inline int ata_ncq_enabled(struct ata_device *dev)
+{
+ return (dev->flags & (ATA_DFLAG_PIO | ATA_DFLAG_NCQ_OFF |
+ ATA_DFLAG_NCQ)) == ATA_DFLAG_NCQ;
+}

/**
* ata_pause - Flush writes and pause 400 nanoseconds.


-
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/