Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 22 Feb 2018 11:18:34 +0000 (UTC)
From:      Warner Losh <imp@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r329819 - in head/sys: cam cam/ata cam/nvme cam/scsi dev/nvme
Message-ID:  <201802221118.w1MBIYQa034679@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: imp
Date: Thu Feb 22 11:18:33 2018
New Revision: 329819
URL: https://svnweb.freebsd.org/changeset/base/329819

Log:
  Backout r329818, r329816 and r329815.
  
  These aren't the commits I thought I was testing prior to
  commit. Revert until I can sort out what happened and fix it.

Modified:
  head/sys/cam/ata/ata_da.c
  head/sys/cam/cam_iosched.c
  head/sys/cam/cam_iosched.h
  head/sys/cam/nvme/nvme_da.c
  head/sys/cam/scsi/scsi_da.c
  head/sys/dev/nvme/nvme.h

Modified: head/sys/cam/ata/ata_da.c
==============================================================================
--- head/sys/cam/ata/ata_da.c	Thu Feb 22 10:55:23 2018	(r329818)
+++ head/sys/cam/ata/ata_da.c	Thu Feb 22 11:18:33 2018	(r329819)
@@ -1705,7 +1705,7 @@ adaregister(struct cam_periph *periph, void *arg)
 	announce_buf = softc->announce_temp;
 	bzero(announce_buf, ADA_ANNOUNCETMP_SZ);
 
-	if (cam_iosched_init(&softc->cam_iosched, periph, 0) != 0) {
+	if (cam_iosched_init(&softc->cam_iosched, periph) != 0) {
 		printf("adaregister: Unable to probe new device. "
 		       "Unable to allocate iosched memory\n");
 		free(softc, M_DEVBUF);

Modified: head/sys/cam/cam_iosched.c
==============================================================================
--- head/sys/cam/cam_iosched.c	Thu Feb 22 10:55:23 2018	(r329818)
+++ head/sys/cam/cam_iosched.c	Thu Feb 22 11:18:33 2018	(r329819)
@@ -68,8 +68,6 @@ static MALLOC_DEFINE(M_CAMSCHED, "CAM I/O Scheduler",
 #define CAM_IOSCHED_FLAG_CALLOUT_ACTIVE (1ul << 1)
 			/* Timer has just ticked */
 #define CAM_IOSCHED_FLAG_TICK		(1ul << 2)
-			/* When set, defer trims until after next tick */
-#define CAM_IOSCHED_FLAG_TRIM_QONLY	(1ul << 4)
 
 			/* Periph drivers set these flags to indicate work */
 #define CAM_IOSCHED_FLAG_WORK_FLAGS	((0xffffu) << 16)
@@ -292,7 +290,6 @@ struct cam_iosched_softc {
 	struct bio_queue_head trim_queue;
 				/* scheduler flags < 16, user flags >= 16 */
 	uint32_t	flags;
-	u_int		caps;
 	int		sort_io_queue;
 #ifdef CAM_IOSCHED_DYNAMIC
 	int		read_bias;		/* Read bias setting */
@@ -590,7 +587,7 @@ cam_iosched_ticker(void *arg)
 	cam_iosched_limiter_tick(&isc->write_stats);
 	cam_iosched_limiter_tick(&isc->trim_stats);
 
-	isc->flags |= CAM_IOSCHED_FLAG_TICK;
+	isc->flags |= CAM_IOSCHED_FLAGS_TICK;
 	cam_iosched_schedule(isc, isc->periph);
 
 	/*
@@ -1067,16 +1064,12 @@ cam_iosched_cl_sysctl_fini(struct control_loop *clp)
  * sizeof struct cam_iosched_softc.
  */
 int
-cam_iosched_init(struct cam_iosched_softc **iscp, struct cam_periph *periph,
-	u_int caps)
+cam_iosched_init(struct cam_iosched_softc **iscp, struct cam_periph *periph)
 {
 
 	*iscp = malloc(sizeof(**iscp), M_CAMSCHED, M_NOWAIT | M_ZERO);
 	if (*iscp == NULL)
 		return ENOMEM;
-	(*iscp)->caps = caps;
-	if (caps & CAM_IOSCHED_CAP_TRIM_CLOCKED)
-		(*iscp)->flags |= CAM_IOSCHED_FLAG_TRIM_QONLY;
 #ifdef CAM_IOSCHED_DYNAMIC
 	if (iosched_debug)
 		printf("CAM IOSCHEDULER Allocating entry at %p\n", *iscp);
@@ -1203,7 +1196,7 @@ cam_iosched_flush(struct cam_iosched_softc *isc, struc
 
 #ifdef CAM_IOSCHED_DYNAMIC
 static struct bio *
-cam_iosched_get_write(struct cam_iosched_softc *isc, bool wastick)
+cam_iosched_get_write(struct cam_iosched_softc *isc)
 {
 	struct bio *bp;
 
@@ -1312,45 +1305,13 @@ cam_iosched_next_trim(struct cam_iosched_softc *isc)
  *
  * Assumes we're called with the periph lock held.
  */
-static struct bio *
-cam_iosched_get_trim(struct cam_iosched_softc *isc, bool wastick)
+struct bio *
+cam_iosched_get_trim(struct cam_iosched_softc *isc)
 {
 
-	/*
-	 * If there's no trims, return NULL. If we're clocking out the
-	 * trims rather than doing thins right away, this is where we
-	 * set the queue only bit. This causes us to ignore them until
-	 * the next clock tick. If we can't get a trim, and we're clocking
-	 * them out, if the queue is empty or if we're rate limited,
-	 * then set QONLY so we stop processing trims until the next
-	 * tick.
-	 */
-	if (!cam_iosched_has_more_trim(isc)) {
-		if ((isc->caps & CAM_IOSCHED_CAP_TRIM_CLOCKED) &&
-		    (bioq_first(&isc->trim_queue) == NULL ||
-#ifdef CAM_IOSCHED_DYNAMIC
-		     (isc->trim_stats.state_flags & IOP_RATE_LIMITED)
-#else
-		     false
-#endif
-		    ))
-			isc->flags |= CAM_IOSCHED_FLAG_TRIM_QONLY;
+	if (!cam_iosched_has_more_trim(isc))
 		return NULL;
-	}
 
-	/*
-	 * If we just ticked, and we have trims, then turn off
-	 * the queue only flag.
-	 */
-	if (wastick)
-		isc->flags &= ~CAM_IOSCHED_FLAG_TRIM_QONLY;
-
-	/*
-	 * If QONLY is set, no trims are eligble just now.
-	 */
-	if (isc->flags & CAM_IOSCHED_FLAG_TRIM_QONLY)
-		return NULL;
-
 	return cam_iosched_next_trim(isc);
 }
 
@@ -1366,17 +1327,17 @@ cam_iosched_next_bio(struct cam_iosched_softc *isc)
 	struct bio *bp;
 	bool wastick;
 	
-	wastick = !!(isc->flags & CAM_IOSCHED_FLAG_TICK);
-	isc->flags &= ~CAM_IOSCHED_FLAG_TICK;
+	wastick = !!(isc->flags & CAM_IOSCHED_FLAGS_TICK);
+	isc->flags &= ~CAM_IOSCHED_FLAGS_TICK;
 
 	/*
 	 * See if we have a trim that can be scheduled. We can only send one
-	 * at a time down, so this takes that into account for those devices
-	 * that can only do one. In addition, some devices queue up a bunch
-	 * of TRIMs before sending them down as a batch.
+	 * at a time down, so this takes that into account.
+	 *
+	 * XXX newer TRIM commands are queueable. Revisit this when we
+	 * implement them.
 	 */
-	if ((isc->flags & CAM_IOSCHED_FLAG_TRIM_QONLY) == 0 &&
-	    (bp = cam_iosched_get_trim(isc, wastick)) != NULL)
+	if ((bp = cam_iosched_get_trim(isc)) != NULL)
 		return bp;
 
 #ifdef CAM_IOSCHED_DYNAMIC
@@ -1385,7 +1346,7 @@ cam_iosched_next_bio(struct cam_iosched_softc *isc)
 	 * and if so, those are next.
 	 */
 	if (do_dynamic_iosched) {
-		if ((bp = cam_iosched_get_write(isc, was_tick)) != NULL)
+		if ((bp = cam_iosched_get_write(isc)) != NULL)
 			return bp;
 	}
 #endif

Modified: head/sys/cam/cam_iosched.h
==============================================================================
--- head/sys/cam/cam_iosched.h	Thu Feb 22 10:55:23 2018	(r329818)
+++ head/sys/cam/cam_iosched.h	Thu Feb 22 11:18:33 2018	(r329819)
@@ -81,12 +81,11 @@ cam_iosched_sbintime_t(uintptr_t delta)
 	return (sbintime_t)((uint64_t)delta << CAM_IOSCHED_TIME_SHIFT);
 }
 
-#define CAM_IOSCHED_CAP_TRIM_CLOCKED	0x1
-
-int cam_iosched_init(struct cam_iosched_softc **, struct cam_periph *periph, u_int caps);
+int cam_iosched_init(struct cam_iosched_softc **, struct cam_periph *periph);
 void cam_iosched_fini(struct cam_iosched_softc *);
 void cam_iosched_sysctl_init(struct cam_iosched_softc *, struct sysctl_ctx_list *, struct sysctl_oid *);
 struct bio *cam_iosched_next_trim(struct cam_iosched_softc *isc);
+struct bio *cam_iosched_get_trim(struct cam_iosched_softc *isc);
 struct bio *cam_iosched_next_bio(struct cam_iosched_softc *isc);
 void cam_iosched_queue_work(struct cam_iosched_softc *isc, struct bio *bp);
 void cam_iosched_flush(struct cam_iosched_softc *isc, struct devstat *stp, int err);

Modified: head/sys/cam/nvme/nvme_da.c
==============================================================================
--- head/sys/cam/nvme/nvme_da.c	Thu Feb 22 10:55:23 2018	(r329818)
+++ head/sys/cam/nvme/nvme_da.c	Thu Feb 22 11:18:33 2018	(r329819)
@@ -122,14 +122,6 @@ struct nda_softc {
 #endif
 };
 
-struct nda_trim_request {
-	union {
-		struct nvme_dsm_range dsm;
-		uint8_t		data[NVME_MAX_DSM_TRIM];
-	} u;
-	TAILQ_HEAD(, bio) bps;
-};
-
 /* Need quirk table */
 
 static	disk_strategy_t	ndastrategy;
@@ -158,14 +150,11 @@ static void		ndasuspend(void *arg);
 #ifndef	NDA_DEFAULT_RETRY
 #define	NDA_DEFAULT_RETRY	4
 #endif
-#ifndef NDA_MAX_TRIM_ENTRIES
-#define NDA_MAX_TRIM_ENTRIES 256	/* Number of DSM trims to use, max 256 */
-#endif
 
+
 //static int nda_retry_count = NDA_DEFAULT_RETRY;
 static int nda_send_ordered = NDA_DEFAULT_SEND_ORDERED;
 static int nda_default_timeout = NDA_DEFAULT_TIMEOUT;
-static int nda_max_trim_entries = NDA_MAX_TRIM_ENTRIES;
 
 /*
  * All NVMe media is non-rotational, so all nvme device instances
@@ -702,8 +691,7 @@ ndaregister(struct cam_periph *periph, void *arg)
 		return(CAM_REQ_CMP_ERR);
 	}
 
-	if (cam_iosched_init(&softc->cam_iosched, periph,
-		CAM_IOSCHED_CAP_TRIM_CLOCKED) != 0) {
+	if (cam_iosched_init(&softc->cam_iosched, periph) != 0) {
 		printf("ndaregister: Unable to probe new device. "
 		       "Unable to allocate iosched memory\n");
 		return(CAM_REQ_CMP_ERR);
@@ -906,40 +894,22 @@ ndastart(struct cam_periph *periph, union ccb *start_c
 		}
 		case BIO_DELETE:
 		{
-			struct nvme_dsm_range *dsm_range, *dsm_end;
-			struct nda_trim_request *trim;
-			struct bio *bp1;
-			int ents;
+			struct nvme_dsm_range *dsm_range;
 
-			trim = malloc(sizeof(*trim), M_NVMEDA, M_ZERO | M_NOWAIT);
-			if (trim == NULL) {
+			dsm_range =
+			    malloc(sizeof(*dsm_range), M_NVMEDA, M_ZERO | M_NOWAIT);
+			if (dsm_range == NULL) {
 				biofinish(bp, NULL, ENOMEM);
 				xpt_release_ccb(start_ccb);
 				ndaschedule(periph);
 				return;
 			}
-			TAILQ_INIT(&trim->bps);
-			bp1 = bp;
-			ents = sizeof(trim->u.data) / sizeof(struct nvme_dsm_range);
-			ents = min(ents, nda_max_trim_entries);
-			dsm_range = &trim->u.dsm;
-			dsm_end = dsm_range + ents;
-			do {
-				TAILQ_INSERT_TAIL(&trim->bps, bp1, bio_queue);
-				dsm_range->length =
-				    bp1->bio_bcount / softc->disk->d_sectorsize;
-				dsm_range->starting_lba =
-				    bp1->bio_offset / softc->disk->d_sectorsize;
-				dsm_range++;
-				if (dsm_range >= dsm_end)
-					break;
-				bp1 = cam_iosched_next_trim(softc->cam_iosched);
-				/* XXX -- Could collapse adjacent ranges, but we don't for now */
-				/* XXX -- Could limit based on total payload size */
-			} while (bp1 != NULL);
-			bp->bio_driver2 = trim;
-			nda_nvme_trim(softc, &start_ccb->nvmeio, &trim->u.dsm,
-			    dsm_range - &trim->u.dsm);
+			dsm_range->length =
+			    bp->bio_bcount / softc->disk->d_sectorsize;
+			dsm_range->starting_lba =
+			    bp->bio_offset / softc->disk->d_sectorsize;
+			bp->bio_driver2 = dsm_range;
+			nda_nvme_trim(softc, &start_ccb->nvmeio, dsm_range, 1);
 			start_ccb->ccb_h.ccb_state = NDA_CCB_TRIM;
 			start_ccb->ccb_h.flags |= CAM_UNLOCKED;
 			/*
@@ -1020,6 +990,8 @@ ndadone(struct cam_periph *periph, union ccb *done_ccb
 		} else {
 			bp->bio_resid = 0;
 		}
+		if (state == NDA_CCB_TRIM)
+			free(bp->bio_driver2, M_NVMEDA);
 		softc->outstanding_cmds--;
 
 		/*
@@ -1031,15 +1003,13 @@ ndadone(struct cam_periph *periph, union ccb *done_ccb
 		cam_iosched_bio_complete(softc->cam_iosched, bp, done_ccb);
 		xpt_release_ccb(done_ccb);
 		if (state == NDA_CCB_TRIM) {
-			struct nda_trim_request *trim;
-			struct bio *bp1;
+#ifdef notyet
 			TAILQ_HEAD(, bio) queue;
+			struct bio *bp1;
 
-			trim = bp->bio_driver2;
 			TAILQ_INIT(&queue);
-			TAILQ_CONCAT(&queue, &trim->bps, bio_queue);
-			free(trim, M_NVMEDA);
-
+			TAILQ_CONCAT(&queue, &softc->trim_req.bps, bio_queue);
+#endif
 			/*
 			 * Since we can have multiple trims in flight, we don't
 			 * need to call this here.
@@ -1047,6 +1017,8 @@ ndadone(struct cam_periph *periph, union ccb *done_ccb
 			 */
 			ndaschedule(periph);
 			cam_periph_unlock(periph);
+#ifdef notyet
+/* Not yet collapsing several BIO_DELETE requests into one TRIM */
 			while ((bp1 = TAILQ_FIRST(&queue)) != NULL) {
 				TAILQ_REMOVE(&queue, bp1, bio_queue);
 				bp1->bio_error = error;
@@ -1057,6 +1029,9 @@ ndadone(struct cam_periph *periph, union ccb *done_ccb
 					bp1->bio_resid = 0;
 				biodone(bp1);
 			}
+#else
+			biodone(bp);
+#endif
 		} else {
 			ndaschedule(periph);
 			cam_periph_unlock(periph);

Modified: head/sys/cam/scsi/scsi_da.c
==============================================================================
--- head/sys/cam/scsi/scsi_da.c	Thu Feb 22 10:55:23 2018	(r329818)
+++ head/sys/cam/scsi/scsi_da.c	Thu Feb 22 11:18:33 2018	(r329819)
@@ -2572,7 +2572,7 @@ daregister(struct cam_periph *periph, void *arg)
 		return(CAM_REQ_CMP_ERR);
 	}
 
-	if (cam_iosched_init(&softc->cam_iosched, periph, 0) != 0) {
+	if (cam_iosched_init(&softc->cam_iosched, periph) != 0) {
 		printf("daregister: Unable to probe new device. "
 		       "Unable to allocate iosched memory\n");
 		free(softc, M_DEVBUF);

Modified: head/sys/dev/nvme/nvme.h
==============================================================================
--- head/sys/dev/nvme/nvme.h	Thu Feb 22 10:55:23 2018	(r329818)
+++ head/sys/dev/nvme/nvme.h	Thu Feb 22 11:18:33 2018	(r329819)
@@ -59,9 +59,6 @@
 /* Cap nvme to 1MB transfers driver explodes with larger sizes */
 #define NVME_MAX_XFER_SIZE		(MAXPHYS < (1<<20) ? MAXPHYS : (1<<20))
 
-/* Largest DSM Trim that can be done */
-#define NVME_MAX_DSM_TRIM		4096
-
 union cap_lo_register {
 	uint32_t	raw;
 	struct {



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