Commit 27fa8262 authored by Linus Torvalds's avatar Linus Torvalds
Browse files
Pull ata fixes from Niklas Cassel:

 - Make sure that the issuing of a deferred non-NCQ command via
   workqueue feature is only used when mixing NCQ and non-NCQ commands
   to the same link (i.e. return value ATA_DEFER_LINK), and nothing
   else. This way we will not incorrectly try to use the feature for
   e.g. PATA drivers

 - The deferred non-NCQ command was stored in a per-port struct. When
   using Port Multipliers with FIS-Based Switching, we would thus
   needlessly defer commands to all other links. Store the deferred QC
   in a per-link struct, such that Port Multipliers with FBS will get
   the same performance as before

 - The issuing of a deferred non-NCQ command via workqueue feature broke
   support for Port Multipliers using Command-Based Switching. The
   issuing of a deferred non-NCQ command via workqueue feature is not
   compatible with the use of ap->excl_link, which PMPs with CBS use for
   fairness (using implicit round robin)

* tag 'ata-7.1-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/libata/linux:
  ata: libata-scsi: do not needlessly defer commands when using PMP with FBS
  ata: libata-scsi: do not use the deferred QC feature on PMPs with CBS
  ata: libata-scsi: do not use the deferred QC feature for ATA_DEFER_PORT
  ata: libata-scsi: improve readability of ata_scsi_qc_issue()
parents 1a2ab0fe 759e8756
Loading
Loading
Loading
Loading
+6 −3
Original line number Diff line number Diff line
@@ -5584,6 +5584,7 @@ void ata_link_init(struct ata_port *ap, struct ata_link *link, int pmp)
	link->pmp = pmp;
	link->active_tag = ATA_TAG_POISON;
	link->hw_sata_spd_limit = UINT_MAX;
	INIT_WORK(&link->deferred_qc_work, ata_scsi_deferred_qc_work);

	/* can't use iterator, ap isn't initialized yet */
	for (i = 0; i < ATA_MAX_DEVICES; i++) {
@@ -5666,7 +5667,6 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
	mutex_init(&ap->scsi_scan_mutex);
	INIT_DELAYED_WORK(&ap->hotplug_task, ata_scsi_hotplug);
	INIT_DELAYED_WORK(&ap->scsi_rescan_task, ata_scsi_dev_rescan);
	INIT_WORK(&ap->deferred_qc_work, ata_scsi_deferred_qc_work);
	INIT_LIST_HEAD(&ap->eh_done_q);
	init_waitqueue_head(&ap->eh_wait_q);
	init_completion(&ap->park_req_pending);
@@ -6291,12 +6291,15 @@ static void ata_port_detach(struct ata_port *ap)

	/* It better be dead now and not have any remaining deferred qc. */
	WARN_ON(!(ap->pflags & ATA_PFLAG_UNLOADED));
	WARN_ON(ap->deferred_qc);

	cancel_work_sync(&ap->deferred_qc_work);
	cancel_delayed_work_sync(&ap->hotplug_task);
	cancel_delayed_work_sync(&ap->scsi_rescan_task);

	ata_for_each_link(link, ap, PMP_FIRST) {
		WARN_ON(link->deferred_qc);
		cancel_work_sync(&link->deferred_qc_work);
	}

	/* Delete port multiplier link transport devices */
	if (ap->pmp_link) {
		int i;
+4 −4
Original line number Diff line number Diff line
@@ -651,11 +651,11 @@ int ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,
			if (qc->scsicmd != scmd)
				continue;
			if ((qc->flags & ATA_QCFLAG_ACTIVE) ||
			    qc == ap->deferred_qc)
			    qc == qc->dev->link->deferred_qc)
				break;
		}

		if (i < ATA_MAX_QUEUE && qc == ap->deferred_qc) {
		if (i < ATA_MAX_QUEUE && qc == qc->dev->link->deferred_qc) {
			/*
			 * This is a deferred command that timed out while
			 * waiting for the command queue to drain. Since the qc
@@ -666,8 +666,8 @@ int ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,
			 * deferred qc work from issuing this qc.
			 */
			WARN_ON_ONCE(qc->flags & ATA_QCFLAG_ACTIVE);
			ap->deferred_qc = NULL;
			cancel_work(&ap->deferred_qc_work);
			qc->dev->link->deferred_qc = NULL;
			cancel_work(&qc->dev->link->deferred_qc_work);
			set_host_byte(scmd, DID_TIME_OUT);
			scsi_eh_finish_cmd(scmd, &ap->eh_done_q);
		} else if (i < ATA_MAX_QUEUE) {
+16 −2
Original line number Diff line number Diff line
@@ -110,13 +110,24 @@ int sata_pmp_qc_defer_cmd_switch(struct ata_queued_cmd *qc)
{
	struct ata_link *link = qc->dev->link;
	struct ata_port *ap = link->ap;
	int ret;

	if (ap->excl_link == NULL || ap->excl_link == link) {
		if (ap->nr_active_links == 0 || ata_link_active(link)) {
			qc->flags |= ATA_QCFLAG_CLEAR_EXCL;
			return ata_std_qc_defer(qc);
			ret = ata_std_qc_defer(qc);
			if (ret == ATA_DEFER_LINK)
				return ATA_DEFER_LINK_EXCL;
			return ret;
		}

		/*
		 * Note: ap->excl_link contains the link that is next in line,
		 * i.e. implicit round robin. If there is only one link
		 * dispatching, ap->excl_link will be left unclaimed, allowing
		 * other links to set ap->excl_link, ensuring that the currently
		 * active link cannot queue any more.
		 */
		ap->excl_link = link;
	}

@@ -571,8 +582,11 @@ static void sata_pmp_detach(struct ata_device *dev)
	if (ap->ops->pmp_detach)
		ap->ops->pmp_detach(ap);

	ata_for_each_link(tlink, ap, EDGE)
	ata_for_each_link(tlink, ap, EDGE) {
		WARN_ON(tlink->deferred_qc);
		cancel_work_sync(&tlink->deferred_qc_work);
		ata_eh_detach_dev(tlink->device);
	}

	spin_lock_irqsave(ap->lock, flags);
	ap->nr_pmp_links = 0;
+52 −37
Original line number Diff line number Diff line
@@ -1664,8 +1664,9 @@ static void ata_scsi_qc_done(struct ata_queued_cmd *qc, bool set_result,

void ata_scsi_deferred_qc_work(struct work_struct *work)
{
	struct ata_port *ap =
		container_of(work, struct ata_port, deferred_qc_work);
	struct ata_link *link =
		container_of(work, struct ata_link, deferred_qc_work);
	struct ata_port *ap = link->ap;
	struct ata_queued_cmd *qc;
	unsigned long flags;

@@ -1676,10 +1677,10 @@ void ata_scsi_deferred_qc_work(struct work_struct *work)
	 * such case, we should not need any more deferring the qc, so warn if
	 * qc_defer() says otherwise.
	 */
	qc = ap->deferred_qc;
	qc = link->deferred_qc;
	if (qc && !ata_port_eh_scheduled(ap)) {
		WARN_ON_ONCE(ap->ops->qc_defer(qc));
		ap->deferred_qc = NULL;
		link->deferred_qc = NULL;
		ata_qc_issue(qc);
	}

@@ -1688,7 +1689,7 @@ void ata_scsi_deferred_qc_work(struct work_struct *work)

void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
{
	struct ata_queued_cmd *qc = ap->deferred_qc;
	struct ata_link *link;

	lockdep_assert_held(ap->lock);

@@ -1697,16 +1698,21 @@ void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
	 * do not try to be smart about what to do with this deferred command
	 * and simply requeue it by completing it with DID_REQUEUE.
	 */
	ata_for_each_link(link, ap, PMP_FIRST) {
		struct ata_queued_cmd *qc = link->deferred_qc;

		if (qc) {
		ap->deferred_qc = NULL;
		cancel_work(&ap->deferred_qc_work);
			link->deferred_qc = NULL;
			cancel_work(&link->deferred_qc_work);
			ata_scsi_qc_done(qc, true, DID_REQUEUE << 16);
		}
	}
}

static void ata_scsi_schedule_deferred_qc(struct ata_port *ap)
static void ata_scsi_schedule_deferred_qc(struct ata_link *link)
{
	struct ata_queued_cmd *qc = ap->deferred_qc;
	struct ata_queued_cmd *qc = link->deferred_qc;
	struct ata_port *ap = link->ap;

	lockdep_assert_held(ap->lock);

@@ -1723,12 +1729,12 @@ static void ata_scsi_schedule_deferred_qc(struct ata_port *ap)
		return;
	}
	if (!ap->ops->qc_defer(qc))
		queue_work(system_highpri_wq, &ap->deferred_qc_work);
		queue_work(system_highpri_wq, &link->deferred_qc_work);
}

static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
{
	struct ata_port *ap = qc->ap;
	struct ata_link *link = qc->dev->link;
	struct scsi_cmnd *cmd = qc->scsicmd;
	u8 *cdb = cmd->cmnd;
	bool have_sense = qc->flags & ATA_QCFLAG_SENSE_VALID;
@@ -1759,22 +1765,23 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)

	ata_scsi_qc_done(qc, false, 0);

	ata_scsi_schedule_deferred_qc(ap);
	ata_scsi_schedule_deferred_qc(link);
}

static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
{
	struct ata_link *link = qc->dev->link;
	int ret;

	if (!ap->ops->qc_defer)
		goto issue;
		goto issue_qc;

	/*
	 * If we already have a deferred qc, then rely on the SCSI layer to
	 * requeue and defer all incoming commands until the deferred qc is
	 * processed, once all on-going commands complete.
	 */
	if (ap->deferred_qc) {
	if (link->deferred_qc) {
		ata_qc_free(qc);
		return SCSI_MLQUEUE_DEVICE_BUSY;
	}
@@ -1786,17 +1793,29 @@ static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
		break;
	case ATA_DEFER_LINK:
		ret = SCSI_MLQUEUE_DEVICE_BUSY;
		break;
		goto defer_qc;
	case ATA_DEFER_LINK_EXCL:
		/*
		 * Drivers making use of ap->excl_link cannot store the QC in
		 * link->deferred_qc, because the ap->excl_link handling is
		 * incompatible with the link->deferred_qc workqueue handling.
		 */
		ret = SCSI_MLQUEUE_DEVICE_BUSY;
		goto free_qc;
	case ATA_DEFER_PORT:
		ret = SCSI_MLQUEUE_HOST_BUSY;
		break;
		goto free_qc;
	default:
		WARN_ON_ONCE(1);
		ret = SCSI_MLQUEUE_HOST_BUSY;
		break;
		goto free_qc;
	}

	if (ret) {
issue_qc:
	ata_qc_issue(qc);
	return 0;

defer_qc:
	/*
	 * We must defer this qc: if this is not an NCQ command, keep
	 * this qc as a deferred one and report to the SCSI layer that
@@ -1805,19 +1824,15 @@ static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
	 * commands complete.
	 */
	if (!ata_is_ncq(qc->tf.protocol)) {
			ap->deferred_qc = qc;
		link->deferred_qc = qc;
		return 0;
	}

free_qc:
	/* Force a requeue of the command to defer its execution. */
	ata_qc_free(qc);
		return ret;
	}

issue:
	ata_qc_issue(qc);

	return 0;
	return ret;
}

/**
+5 −1
Original line number Diff line number Diff line
@@ -789,6 +789,7 @@ static int sil24_qc_defer(struct ata_queued_cmd *qc)
	struct ata_link *link = qc->dev->link;
	struct ata_port *ap = link->ap;
	u8 prot = qc->tf.protocol;
	int ret;

	/*
	 * There is a bug in the chip:
@@ -826,7 +827,10 @@ static int sil24_qc_defer(struct ata_queued_cmd *qc)
		qc->flags |= ATA_QCFLAG_CLEAR_EXCL;
	}

	return ata_std_qc_defer(qc);
	ret = ata_std_qc_defer(qc);
	if (ret == ATA_DEFER_LINK)
		return ATA_DEFER_LINK_EXCL;
	return ret;
}

static enum ata_completion_errors sil24_qc_prep(struct ata_queued_cmd *qc)
Loading