Date: Wed, 17 Oct 2012 12:48:56 -0500 (CDT) From: Bryan Venteicher <bryanv@daemoninthecloset.org> To: freebsd-scsi@freebsd.org Subject: bioq_disksort() on SSDs Message-ID: <1780024492.108.1350496136356.JavaMail.root@daemoninthecloset.org> In-Reply-To: <31199212.85.1350495039233.JavaMail.root@daemoninthecloset.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
Hi,
At $JOB, we've noticed bioq_disksort() creep up in the profiling
output when the bio queue gets backed up at our caching SSD drives.
Thoughts on the attached patch which does away with the disksort
for SCSI da* and ATA ada* SSD drives?
Support for quirks and sysctl/tunables to force the behavior either
way are desirable but not implemented in this patch. Nor is some
mechanism to pass this information up to GEOM (to say gsched). And
perhaps this should done fairer to writes as DragonflyBSD's
bioqdisksort() does.
(I've only been able to test an similar patch adapted to 8.x and this
patch on a virtual machine.)
http://www.daemoninthecloset.org/~bryanv/patches/freebsd/ssd_disksort.patch
[-- Attachment #2 --]
diff --git a/sys/cam/ata/ata_da.c b/sys/cam/ata/ata_da.c
index bb63ac8..fe32c79 100644
--- a/sys/cam/ata/ata_da.c
+++ b/sys/cam/ata/ata_da.c
@@ -75,18 +75,19 @@ typedef enum {
} ada_state;
typedef enum {
- ADA_FLAG_PACK_INVALID = 0x001,
- ADA_FLAG_CAN_48BIT = 0x002,
- ADA_FLAG_CAN_FLUSHCACHE = 0x004,
- ADA_FLAG_CAN_NCQ = 0x008,
- ADA_FLAG_CAN_DMA = 0x010,
- ADA_FLAG_NEED_OTAG = 0x020,
- ADA_FLAG_WENT_IDLE = 0x040,
- ADA_FLAG_CAN_TRIM = 0x080,
- ADA_FLAG_OPEN = 0x100,
- ADA_FLAG_SCTX_INIT = 0x200,
- ADA_FLAG_CAN_CFA = 0x400,
- ADA_FLAG_CAN_POWERMGT = 0x800
+ ADA_FLAG_PACK_INVALID = 0x0001,
+ ADA_FLAG_CAN_48BIT = 0x0002,
+ ADA_FLAG_CAN_FLUSHCACHE = 0x0004,
+ ADA_FLAG_CAN_NCQ = 0x0008,
+ ADA_FLAG_CAN_DMA = 0x0010,
+ ADA_FLAG_NEED_OTAG = 0x0020,
+ ADA_FLAG_WENT_IDLE = 0x0040,
+ ADA_FLAG_CAN_TRIM = 0x0080,
+ ADA_FLAG_OPEN = 0x0100,
+ ADA_FLAG_SCTX_INIT = 0x0200,
+ ADA_FLAG_CAN_CFA = 0x0400,
+ ADA_FLAG_CAN_POWERMGT = 0x0800,
+ ADA_FLAG_NO_DISKSORT = 0x1000
} ada_flags;
typedef enum {
@@ -535,6 +536,7 @@ adastrategy(struct bio *bp)
{
struct cam_periph *periph;
struct ada_softc *softc;
+ struct bio_queue_head *bioq;
periph = (struct cam_periph *)bp->bio_disk->d_drv1;
if (periph == NULL) {
@@ -559,11 +561,14 @@ adastrategy(struct bio *bp)
/*
* Place it in the queue of disk activities for this disk
*/
- if (bp->bio_cmd == BIO_DELETE &&
- (softc->flags & ADA_FLAG_CAN_TRIM))
- bioq_disksort(&softc->trim_queue, bp);
+ if (bp->bio_cmd == BIO_DELETE && (softc->flags & ADA_FLAG_CAN_TRIM))
+ bioq = &softc->trim_queue;
else
- bioq_disksort(&softc->bio_queue, bp);
+ bioq = &softc->bio_queue;
+ if (softc->flags & ADA_FLAG_NO_DISKSORT)
+ bioq_insert_tail(bioq, bp);
+ else
+ bioq_disksort(bioq, bp);
/*
* Schedule ourselves for performing the work.
@@ -985,6 +990,9 @@ adaregister(struct cam_periph *periph, void *arg)
if (cgd->ident_data.support.command2 & ATA_SUPPORT_CFA)
softc->flags |= ADA_FLAG_CAN_CFA;
+ if (cgd->ident_data.media_rotation_rate == 1)
+ softc->flags |= ADA_FLAG_NO_DISKSORT;
+
periph->softc = softc;
/*
diff --git a/sys/cam/scsi/scsi_all.c b/sys/cam/scsi/scsi_all.c
index a37ae19..1e55f8b 100644
--- a/sys/cam/scsi/scsi_all.c
+++ b/sys/cam/scsi/scsi_all.c
@@ -40,6 +40,9 @@ __FBSDID("$FreeBSD$");
#include <sys/systm.h>
#include <sys/libkern.h>
#include <sys/kernel.h>
+#include <sys/lock.h>
+#include <sys/malloc.h>
+#include <sys/mutex.h>
#include <sys/sysctl.h>
#else
#include <errno.h>
@@ -51,7 +54,11 @@ __FBSDID("$FreeBSD$");
#include <cam/cam.h>
#include <cam/cam_ccb.h>
#include <cam/cam_queue.h>
+#include <cam/cam_periph.h>
#include <cam/cam_xpt.h>
+#include <cam/cam_xpt_sim.h>
+#include <cam/cam_xpt_periph.h>
+#include <cam/cam_xpt_internal.h>
#include <cam/scsi/scsi_all.h>
#include <sys/sbuf.h>
#ifndef _KERNEL
@@ -6159,6 +6166,28 @@ scsi_devid_match(uint8_t *lhs, size_t lhs_len, uint8_t *rhs, size_t rhs_len)
return (-1);
}
+int
+scsi_vpd_page_supported(struct cam_periph *periph, uint8_t page_id)
+{
+ struct cam_ed *device;
+ struct scsi_vpd_supported_pages *vpds;
+ int i, num_pages;
+
+ device = periph->path->device;
+ vpds = (struct scsi_vpd_supported_pages *)device->supported_vpds;
+
+ if (vpds != NULL) {
+ num_pages = device->supported_vpds_len -
+ SVPD_SUPPORTED_PAGES_HDR_LEN;
+ for (i = 0; i < num_pages; i++) {
+ if (vpds->page_list[i] == page_id)
+ return (1);
+ }
+ }
+
+ return (0);
+}
+
#ifdef _KERNEL
static void
init_scsi_delay(void)
diff --git a/sys/cam/scsi/scsi_all.h b/sys/cam/scsi/scsi_all.h
index 0693e1c..8c2cf41 100644
--- a/sys/cam/scsi/scsi_all.h
+++ b/sys/cam/scsi/scsi_all.h
@@ -1392,6 +1392,15 @@ struct scsi_vpd_id_scsi_name
uint8_t name_string[256];
};
+struct scsi_vpd_block_characteristics
+{
+ uint8_t device;
+ uint8_t page_code;
+#define SVPD_BLOCK_CHARACTERISTICS 0xB1
+ uint8_t length[2];
+ uint8_t rotation_rate[2];
+};
+
struct scsi_service_action_in
{
uint8_t opcode;
@@ -2387,6 +2396,9 @@ int scsi_static_inquiry_match(caddr_t inqbuffer,
int scsi_devid_match(uint8_t *rhs, size_t rhs_len,
uint8_t *lhs, size_t lhs_len);
+int scsi_vpd_page_supported(struct cam_periph *periph,
+ uint8_t page_id);
+
void scsi_extract_sense(struct scsi_sense_data *sense, int *error_code,
int *sense_key, int *asc, int *ascq);
int scsi_extract_sense_ccb(union ccb *ccb, int *error_code, int *sense_key,
diff --git a/sys/cam/scsi/scsi_da.c b/sys/cam/scsi/scsi_da.c
index aea2cd3..9dc96b4 100644
--- a/sys/cam/scsi/scsi_da.c
+++ b/sys/cam/scsi/scsi_da.c
@@ -73,18 +73,19 @@ typedef enum {
} da_state;
typedef enum {
- DA_FLAG_PACK_INVALID = 0x001,
- DA_FLAG_NEW_PACK = 0x002,
- DA_FLAG_PACK_LOCKED = 0x004,
- DA_FLAG_PACK_REMOVABLE = 0x008,
- DA_FLAG_SAW_MEDIA = 0x010,
- DA_FLAG_NEED_OTAG = 0x020,
- DA_FLAG_WENT_IDLE = 0x040,
- DA_FLAG_RETRY_UA = 0x080,
- DA_FLAG_OPEN = 0x100,
- DA_FLAG_SCTX_INIT = 0x200,
- DA_FLAG_CAN_RC16 = 0x400,
- DA_FLAG_PROBED = 0x800
+ DA_FLAG_PACK_INVALID = 0x0001,
+ DA_FLAG_NEW_PACK = 0x0002,
+ DA_FLAG_PACK_LOCKED = 0x0004,
+ DA_FLAG_PACK_REMOVABLE = 0x0008,
+ DA_FLAG_SAW_MEDIA = 0x0010,
+ DA_FLAG_NEED_OTAG = 0x0020,
+ DA_FLAG_WENT_IDLE = 0x0040,
+ DA_FLAG_RETRY_UA = 0x0080,
+ DA_FLAG_OPEN = 0x0100,
+ DA_FLAG_SCTX_INIT = 0x0200,
+ DA_FLAG_CAN_RC16 = 0x0400,
+ DA_FLAG_PROBED = 0x0800,
+ DA_FLAG_NO_DISKSORT = 0x1000,
} da_flags;
typedef enum {
@@ -855,6 +856,7 @@ static int daerror(union ccb *ccb, u_int32_t cam_flags,
u_int32_t sense_flags);
static void daprevent(struct cam_periph *periph, int action);
static void dareprobe(struct cam_periph *periph);
+static void daissolidstate(struct cam_periph *periph);
static void dasetgeom(struct cam_periph *periph, uint32_t block_len,
uint64_t maxsector,
struct scsi_read_capacity_data_long *rcaplong,
@@ -980,6 +982,9 @@ daopen(struct disk *dp)
daprevent(periph, PR_PREVENT);
if (error == 0)
+ daissolidstate(periph);
+
+ if (error == 0)
softc->flags |= DA_FLAG_SAW_MEDIA;
cam_periph_unhold(periph);
@@ -1086,6 +1091,7 @@ dastrategy(struct bio *bp)
{
struct cam_periph *periph;
struct da_softc *softc;
+ struct bio_queue_head *bioq;
periph = (struct cam_periph *)bp->bio_disk->d_drv1;
if (periph == NULL) {
@@ -1111,16 +1117,24 @@ dastrategy(struct bio *bp)
* Place it in the queue of disk activities for this disk
*/
if (bp->bio_cmd == BIO_DELETE) {
- if (bp->bio_bcount == 0)
+ if (bp->bio_bcount == 0) {
biodone(bp);
- else
- bioq_disksort(&softc->delete_queue, bp);
+ goto schedule;
+ }
+
+ bioq = &softc->delete_queue;
} else
- bioq_disksort(&softc->bio_queue, bp);
+ bioq = &softc->bio_queue;
+
+ if (softc->flags & DA_FLAG_NO_DISKSORT)
+ bioq_insert_tail(bioq, bp);
+ else
+ bioq_disksort(bioq, bp);
/*
* Schedule ourselves for performing the work.
*/
+schedule:
daschedule(periph);
cam_periph_unlock(periph);
@@ -2539,6 +2553,48 @@ dareprobe(struct cam_periph *periph)
xpt_schedule(periph, CAM_PRIORITY_DEV);
}
+static void
+daissolidstate(struct cam_periph *periph)
+{
+ struct da_softc *softc;
+ struct scsi_vpd_block_characteristics *vpd;
+ union ccb *ccb;
+ int error;
+
+ softc = (struct da_softc *)periph->softc;
+
+ if (scsi_vpd_page_supported(periph, SVPD_BLOCK_CHARACTERISTICS) == 0)
+ return;
+
+ vpd = malloc(sizeof(*vpd), M_SCSIDA, M_NOWAIT|M_ZERO);
+ if (vpd == NULL) {
+ xpt_print(periph->path,
+ "Cannot allocate block characteristics VPD page\n");
+ return;
+ }
+
+ ccb = cam_periph_getccb(periph, CAM_PRIORITY_NORMAL);
+ scsi_inquiry(&ccb->csio,
+ /*retries*/4,
+ dadone,
+ MSG_SIMPLE_Q_TAG,
+ (uint8_t *)vpd,
+ sizeof(*vpd),
+ /*evpd*/TRUE,
+ SVPD_BLOCK_CHARACTERISTICS,
+ SSD_MIN_SIZE,
+ /*timeout*/60 * 1000);
+
+ error = cam_periph_runccb(ccb, daerror, CAM_RETRY_SELTO,
+ SF_RETRY_UA | SF_QUIET_IR, softc->disk->d_devstat);
+ xpt_release_ccb(ccb);
+
+ if (error == 0 && scsi_2btoul(vpd->rotation_rate) == 1)
+ softc->flags |= DA_FLAG_NO_DISKSORT;
+
+ free(vpd, M_SCSIDA);
+}
+
static int
daerror(union ccb *ccb, u_int32_t cam_flags, u_int32_t sense_flags)
{
diff --git a/sys/cam/scsi/scsi_xpt.c b/sys/cam/scsi/scsi_xpt.c
index c381efc..02ae679 100644
--- a/sys/cam/scsi/scsi_xpt.c
+++ b/sys/cam/scsi/scsi_xpt.c
@@ -556,7 +556,6 @@ static const int scsi_quirk_table_size =
static cam_status proberegister(struct cam_periph *periph,
void *arg);
static void probeschedule(struct cam_periph *probe_periph);
-static int device_has_vpd(struct cam_ed *device, uint8_t page_id);
static void probestart(struct cam_periph *periph, union ccb *start_ccb);
static void proberequestdefaultnegotiation(struct cam_periph *periph);
static int proberequestbackoff(struct cam_periph *periph,
@@ -708,21 +707,6 @@ probeschedule(struct cam_periph *periph)
xpt_schedule(periph, CAM_PRIORITY_XPT);
}
-static int
-device_has_vpd(struct cam_ed *device, uint8_t page_id)
-{
- int i, num_pages;
- struct scsi_vpd_supported_pages *vpds;
-
- vpds = (struct scsi_vpd_supported_pages *)device->supported_vpds;
- num_pages = device->supported_vpds_len - SVPD_SUPPORTED_PAGES_HDR_LEN;
- for (i = 0;i < num_pages;i++)
- if (vpds->page_list[i] == page_id)
- return 1;
-
- return 0;
-}
-
static void
probestart(struct cam_periph *periph, union ccb *start_ccb)
{
@@ -910,11 +894,9 @@ again:
case PROBE_DEVICE_ID:
{
struct scsi_vpd_device_id *devid;
- struct cam_ed *device;
devid = NULL;
- device = periph->path->device;
- if (device_has_vpd(device, SVPD_DEVICE_ID))
+ if (scsi_vpd_page_supported(periph, SVPD_DEVICE_ID))
devid = malloc(SVPD_DEVICE_ID_MAX_SIZE, M_CAMXPT,
M_NOWAIT | M_ZERO);
@@ -952,7 +934,7 @@ again:
device->serial_num_len = 0;
}
- if (device_has_vpd(device, SVPD_UNIT_SERIAL_NUMBER))
+ if (scsi_vpd_page_supported(periph, SVPD_UNIT_SERIAL_NUMBER))
serial_buf = (struct scsi_vpd_unit_serial_number *)
malloc(sizeof(*serial_buf), M_CAMXPT,
M_NOWAIT|M_ZERO);
@@ -3056,4 +3038,3 @@ scsi_announce_periph(struct cam_periph *periph)
}
printf("\n");
}
-
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1780024492.108.1350496136356.JavaMail.root>