Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 12 Oct 2013 18:24:52 +0000 (UTC)
From:      Alexander Motin <mav@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-projects@freebsd.org
Subject:   svn commit: r256388 - projects/camlock/sys/dev/ahci
Message-ID:  <201310121824.r9CIOq66079232@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mav
Date: Sat Oct 12 18:24:52 2013
New Revision: 256388
URL: http://svnweb.freebsd.org/changeset/base/256388

Log:
  Set of optimizations for ahci(4), respecting CAM improvements:
   - Add tunable hint.ahci.X.direct to control when to use direct request
  completion and when prefer to use CAM threads.  Set default based on number
  of supported MSI vectors and implemented ports.
   - Move heavy AHCI_P_IS read out of the lock to reduce its scope.  This
  reduces lock congestion spinning under high IOPS from 10% to almost zero.
   - Create several optimized interrupt handlers for different configurations
  to avoid unneeded branching there at run time.
  
  At this point I can totally max out 6 SSDs on ICH10 AHCI doing 640K IOPS,
  while having only 30% of CPU load on 2x6x2 CPU Xeon E5645 system.

Modified:
  projects/camlock/sys/dev/ahci/ahci.c
  projects/camlock/sys/dev/ahci/ahci.h

Modified: projects/camlock/sys/dev/ahci/ahci.c
==============================================================================
--- projects/camlock/sys/dev/ahci/ahci.c	Sat Oct 12 17:46:13 2013	(r256387)
+++ projects/camlock/sys/dev/ahci/ahci.c	Sat Oct 12 18:24:52 2013	(r256388)
@@ -55,6 +55,7 @@ __FBSDID("$FreeBSD$");
 static int ahci_setup_interrupt(device_t dev);
 static void ahci_intr(void *data);
 static void ahci_intr_one(void *data);
+static void ahci_intr_one_edge(void *data);
 static int ahci_suspend(device_t dev);
 static int ahci_resume(device_t dev);
 static int ahci_ch_init(device_t dev);
@@ -62,8 +63,9 @@ static int ahci_ch_deinit(device_t dev);
 static int ahci_ch_suspend(device_t dev);
 static int ahci_ch_resume(device_t dev);
 static void ahci_ch_pm(void *arg);
-static void ahci_ch_intr_locked(void *data);
-static void ahci_ch_intr(void *data);
+static void ahci_ch_intr(void *arg);
+static void ahci_ch_intr_direct(void *arg);
+static void ahci_ch_intr_main(struct ahci_channel *ch, uint32_t istatus);
 static int ahci_ctlr_reset(device_t dev);
 static int ahci_ctlr_setup(device_t dev);
 static void ahci_begin_transaction(device_t dev, union ccb *ccb);
@@ -397,6 +399,7 @@ ahci_attach(device_t dev)
 	struct ahci_controller *ctlr = device_get_softc(dev);
 	device_t child;
 	int	error, unit, speed, i;
+	u_int	u;
 	uint32_t devid = pci_get_devid(dev);
 	uint8_t revid = pci_get_revid(dev);
 	u_int32_t version;
@@ -496,6 +499,12 @@ ahci_attach(device_t dev)
 		rman_fini(&ctlr->sc_iomem);
 		return ENXIO;
 	}
+	i = 0;
+	for (u = ctlr->ichannels; u != 0; u >>= 1)
+		i += (u & 1);
+	ctlr->direct = (ctlr->msi && (ctlr->numirqs > 1 || i <= 3));
+	resource_int_value(device_get_name(dev), device_get_unit(dev),
+	    "direct", &ctlr->direct);
 	/* Announce HW capabilities. */
 	speed = (ctlr->caps & AHCI_CAP_ISS) >> AHCI_CAP_ISS_SHIFT;
 	device_printf(dev,
@@ -677,24 +686,26 @@ static int
 ahci_setup_interrupt(device_t dev)
 {
 	struct ahci_controller *ctlr = device_get_softc(dev);
-	int i, msi = 2;
+	int i;
 
+	ctlr->msi = 2;
 	/* Process hints. */
 	if (ctlr->quirks & AHCI_Q_NOMSI)
-		msi = 0;
+		ctlr->msi = 0;
 	resource_int_value(device_get_name(dev),
-	    device_get_unit(dev), "msi", &msi);
-	if (msi < 0)
-		msi = 0;
-	else if (msi == 1)
-		msi = min(1, pci_msi_count(dev));
-	else if (msi > 1)
-		msi = pci_msi_count(dev);
+	    device_get_unit(dev), "msi", &ctlr->msi);
+	ctlr->numirqs = 1;
+	if (ctlr->msi < 0)
+		ctlr->msi = 0;
+	else if (ctlr->msi == 1)
+		ctlr->msi = min(1, pci_msi_count(dev));
+	else if (ctlr->msi > 1) {
+		ctlr->msi = 2;
+		ctlr->numirqs = pci_msi_count(dev);
+	}
 	/* Allocate MSI if needed/present. */
-	if (msi && pci_alloc_msi(dev, &msi) == 0) {
-		ctlr->numirqs = msi;
-	} else {
-		msi = 0;
+	if (ctlr->msi && pci_alloc_msi(dev, &ctlr->numirqs) != 0) {
+		ctlr->msi = 0;
 		ctlr->numirqs = 1;
 	}
 	/* Check for single MSI vector fallback. */
@@ -706,7 +717,7 @@ ahci_setup_interrupt(device_t dev)
 	/* Allocate all IRQs. */
 	for (i = 0; i < ctlr->numirqs; i++) {
 		ctlr->irqs[i].ctlr = ctlr;
-		ctlr->irqs[i].r_irq_rid = i + (msi ? 1 : 0);
+		ctlr->irqs[i].r_irq_rid = i + (ctlr->msi ? 1 : 0);
 		if (ctlr->numirqs == 1 || i >= ctlr->channels ||
 		    (ctlr->ccc && i == ctlr->cccv))
 			ctlr->irqs[i].mode = AHCI_IRQ_MODE_ALL;
@@ -720,7 +731,9 @@ ahci_setup_interrupt(device_t dev)
 			return ENXIO;
 		}
 		if ((bus_setup_intr(dev, ctlr->irqs[i].r_irq, ATA_INTR_FLAGS, NULL,
-		    (ctlr->irqs[i].mode == AHCI_IRQ_MODE_ONE) ? ahci_intr_one : ahci_intr,
+		    (ctlr->irqs[i].mode != AHCI_IRQ_MODE_ONE) ? ahci_intr :
+		     ((ctlr->quirks & AHCI_Q_EDGEIS) ? ahci_intr_one_edge :
+		      ahci_intr_one),
 		    &ctlr->irqs[i], &ctlr->irqs[i].handle))) {
 			/* SOS XXX release r_irq */
 			device_printf(dev, "unable to setup interrupt\n");
@@ -789,14 +802,25 @@ ahci_intr_one(void *data)
 	int unit;
 
 	unit = irq->r_irq_rid - 1;
-	/* Some controllers have edge triggered IS. */
-	if (ctlr->quirks & AHCI_Q_EDGEIS)
-		ATA_OUTL(ctlr->r_mem, AHCI_IS, 1 << unit);
 	if ((arg = ctlr->interrupt[unit].argument))
 	    ctlr->interrupt[unit].function(arg);
 	/* AHCI declares level triggered IS. */
-	if (!(ctlr->quirks & AHCI_Q_EDGEIS))
-		ATA_OUTL(ctlr->r_mem, AHCI_IS, 1 << unit);
+	ATA_OUTL(ctlr->r_mem, AHCI_IS, 1 << unit);
+}
+
+static void
+ahci_intr_one_edge(void *data)
+{
+	struct ahci_controller_irq *irq = data;
+	struct ahci_controller *ctlr = irq->ctlr;
+	void *arg;
+	int unit;
+
+	unit = irq->r_irq_rid - 1;
+	/* Some controllers have edge triggered IS. */
+	ATA_OUTL(ctlr->r_mem, AHCI_IS, 1 << unit);
+	if ((arg = ctlr->interrupt[unit].argument))
+		ctlr->interrupt[unit].function(arg);
 }
 
 static struct resource *
@@ -1046,7 +1070,8 @@ ahci_ch_attach(device_t dev)
 		goto err0;
 	}
 	if ((bus_setup_intr(dev, ch->r_irq, ATA_INTR_FLAGS, NULL,
-	    ahci_ch_intr_locked, dev, &ch->ih))) {
+	    ctlr->direct ? ahci_ch_intr_direct : ahci_ch_intr,
+	    dev, &ch->ih))) {
 		device_printf(dev, "Unable to setup interrupt\n");
 		error = ENXIO;
 		goto err1;
@@ -1480,15 +1505,38 @@ ahci_done(struct ahci_channel *ch, union
 }
 
 static void
-ahci_ch_intr_locked(void *data)
+ahci_ch_intr(void *arg)
+{
+	device_t dev = (device_t)arg;
+	struct ahci_channel *ch = device_get_softc(dev);
+	uint32_t istatus;
+
+	/* Read interrupt statuses. */
+	istatus = ATA_INL(ch->r_mem, AHCI_P_IS);
+	if (istatus == 0)
+		return;
+
+	mtx_lock(&ch->mtx);
+	ahci_ch_intr_main(ch, istatus);
+	mtx_unlock(&ch->mtx);
+}
+
+static void
+ahci_ch_intr_direct(void *arg)
 {
-	device_t dev = (device_t)data;
+	device_t dev = (device_t)arg;
 	struct ahci_channel *ch = device_get_softc(dev);
 	struct ccb_hdr *ccb_h;
+	uint32_t istatus;
+
+	/* Read interrupt statuses. */
+	istatus = ATA_INL(ch->r_mem, AHCI_P_IS);
+	if (istatus == 0)
+		return;
 
 	mtx_lock(&ch->mtx);
 	ch->batch = 1;
-	ahci_ch_intr(data);
+	ahci_ch_intr_main(ch, istatus);
 	ch->batch = 0;
 	mtx_unlock(&ch->mtx);
 	while ((ccb_h = STAILQ_FIRST(&ch->doneq)) != NULL) {
@@ -1515,18 +1563,14 @@ ahci_ch_pm(void *arg)
 }
 
 static void
-ahci_ch_intr(void *data)
+ahci_ch_intr_main(struct ahci_channel *ch, uint32_t istatus)
 {
-	device_t dev = (device_t)data;
-	struct ahci_channel *ch = device_get_softc(dev);
-	uint32_t istatus, cstatus, serr = 0, sntf = 0, ok, err;
+	device_t dev = ch->dev;
+	uint32_t cstatus, serr = 0, sntf = 0, ok, err;
 	enum ahci_err_type et;
 	int i, ccs, port, reset = 0;
 
-	/* Read and clear interrupt statuses. */
-	istatus = ATA_INL(ch->r_mem, AHCI_P_IS);
-	if (istatus == 0)
-		return;
+	/* Clear interrupt statuses. */
 	ATA_OUTL(ch->r_mem, AHCI_P_IS, istatus);
 	/* Read command statuses. */
 	if (ch->numtslots != 0)
@@ -3019,8 +3063,12 @@ static void
 ahcipoll(struct cam_sim *sim)
 {
 	struct ahci_channel *ch = (struct ahci_channel *)cam_sim_softc(sim);
+	uint32_t istatus;
 
-	ahci_ch_intr(ch->dev);
+	/* Read interrupt statuses and process if any. */
+	istatus = ATA_INL(ch->r_mem, AHCI_P_IS);
+	if (istatus != 0)
+		ahci_ch_intr_main(ch, istatus);
 	if (ch->resetting != 0 &&
 	    (--ch->resetpolldiv <= 0 || !callout_pending(&ch->reset_timer))) {
 		ch->resetpolldiv = 1000;

Modified: projects/camlock/sys/dev/ahci/ahci.h
==============================================================================
--- projects/camlock/sys/dev/ahci/ahci.h	Sat Oct 12 17:46:13 2013	(r256387)
+++ projects/camlock/sys/dev/ahci/ahci.h	Sat Oct 12 18:24:52 2013	(r256388)
@@ -483,6 +483,8 @@ struct ahci_controller {
 	int			ichannels;
 	int			ccc;		/* CCC timeout */
 	int			cccv;		/* CCC vector */
+	int			direct;		/* Direct command completion */
+	int			msi;		/* MSI interupts */
 	struct {
 		void			(*function)(void *);
 		void			*argument;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201310121824.r9CIOq66079232>