Module Name: src
Committed By: jdolecek
Date: Fri Jun 23 20:40:51 UTC 2017
Modified Files:
src/sys/dev/ata [jdolecek-ncq]: TODO.ncq ata.c atavar.h wd.c wdvar.h
Log Message:
restart I/O processing after freeing xfer, i.e. now even after commands
like cache flush or standby; the command handling no longer use on-stack xfer,
hence use queue slot and compete with normal I/O for the xfers
the restart give change to all drives attached to the same channel
in round-robin fashion, for fair usage and to recover from situation
when disk is idle due to all xfers being consumed by other drives
make special concession for flush cache - ignore any new I/O requests
on the particular disk while the flush cache is waiting for xfer,
so that I/O queue won't starve the flush cache and the flush cache would
be done ASAP
tested on piixide(4), ahci(4), siisata(4)
To generate a diff of this commit:
cvs rdiff -u -r1.1.2.21 -r1.1.2.22 src/sys/dev/ata/TODO.ncq
cvs rdiff -u -r1.132.8.15 -r1.132.8.16 src/sys/dev/ata/ata.c
cvs rdiff -u -r1.92.8.14 -r1.92.8.15 src/sys/dev/ata/atavar.h
cvs rdiff -u -r1.428.2.19 -r1.428.2.20 src/sys/dev/ata/wd.c
cvs rdiff -u -r1.43.4.5 -r1.43.4.6 src/sys/dev/ata/wdvar.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/ata/TODO.ncq
diff -u src/sys/dev/ata/TODO.ncq:1.1.2.21 src/sys/dev/ata/TODO.ncq:1.1.2.22
--- src/sys/dev/ata/TODO.ncq:1.1.2.21 Wed Jun 21 22:40:43 2017
+++ src/sys/dev/ata/TODO.ncq Fri Jun 23 20:40:51 2017
@@ -2,12 +2,15 @@ Bugs
----
fix crashdump for mvsata - request times out (maybe same as HEAD?)
+- HEAD crash due to KASSERT(chp->ch_queue->active_xfer != NULL)
-siisata - fix all new XXX and unmergable bits, fix PMP
+siisata - fix all new XXX and unmergable bits
test crashdump with siisata
- fails with recursive panic via pmap_kremove_local() regardless if
- drive connected via PMP or direct
+ drive connected via PMP or direct (failed KASSERT(panicstr != NULL))
+- HEAD crashes same as branch
+- see kern/49610 for potential fix
test wd* at umass?, confirm the ata_channel kludge works
+ add detach code (channel detach, free queue)
@@ -19,13 +22,16 @@ do proper NCQ error recovery (currently
maybe do device error handling in not-interrupt-context (maybe this should be
done on a mpata branch?)
-add mechanics to re-check queue when xfer is finished - needed on PMP
-and for IDE disks, when one drive uses up all available xfers nothing
-ever restarts queue for the other drives
+do not limit openings for xfers for non-NCQ drives in wdstart(), the tag
+will not be used so can use any xfer
+
+in atastart(), restrict NCQ commands to commands for the same drive? it's
+fine for fis-based switching to have outstanding for several drives, but
+not non-FIS
Other random notes (do outside the NCQ branch):
-----------------------------------------------------
-change wd(4) to use dk_open()
+change wd(4) to use dksubr
dump to unopened disk fails (e.g. dump do wd1b when wd1a not mounted), due
to the open path executing ata_get_params(), which eventually tsleeps()
Index: src/sys/dev/ata/ata.c
diff -u src/sys/dev/ata/ata.c:1.132.8.15 src/sys/dev/ata/ata.c:1.132.8.16
--- src/sys/dev/ata/ata.c:1.132.8.15 Wed Jun 21 22:40:43 2017
+++ src/sys/dev/ata/ata.c Fri Jun 23 20:40:51 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: ata.c,v 1.132.8.15 2017/06/21 22:40:43 jdolecek Exp $ */
+/* $NetBSD: ata.c,v 1.132.8.16 2017/06/23 20:40:51 jdolecek Exp $ */
/*
* Copyright (c) 1998, 2001 Manuel Bouyer. All rights reserved.
@@ -25,7 +25,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.132.8.15 2017/06/21 22:40:43 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.132.8.16 2017/06/23 20:40:51 jdolecek Exp $");
#include "opt_ata.h"
@@ -2143,3 +2143,47 @@ atacmd_toncq(struct ata_xfer *xfer, uint
if (xfer->c_bio.flags & ATA_FUA)
*device |= WDSD_FUA;
}
+
+/*
+ * Must be called without any locks, i.e. with both drive and channel locks
+ * released.
+ */
+void
+ata_channel_start(struct ata_channel *chp, int drive)
+{
+ int i;
+ struct ata_drive_datas *drvp;
+
+ KASSERT(chp->ch_ndrives > 0);
+
+#define ATA_DRIVE_START(chp, drive) \
+ do { \
+ KASSERT(drive < chp->ch_ndrives); \
+ drvp = &chp->ch_drive[drive]; \
+ \
+ if (drvp->drive_type != ATA_DRIVET_ATA && \
+ drvp->drive_type != ATA_DRIVET_ATAPI && \
+ drvp->drive_type != ATA_DRIVET_OLD) \
+ continue; \
+ \
+ if (drvp->drv_start != NULL) \
+ (*drvp->drv_start)(drvp->drv_softc); \
+ } while (0)
+
+ /*
+ * Process drives in round robin fashion starting with next one after
+ * the one which finished transfer. Thus no single drive would
+ * completely starve other drives on same channel.
+ * This loop processes all but the current drive, so won't do anything
+ * if there is only one drive in channel.
+ */
+ for (i = (drive + 1) % chp->ch_ndrives; i != drive;
+ i = (i + 1) % chp->ch_ndrives) {
+ ATA_DRIVE_START(chp, i);
+ }
+
+ /* Now try to kick off xfers on the current drive */
+ ATA_DRIVE_START(chp, drive);
+
+#undef ATA_DRIVE_START
+}
Index: src/sys/dev/ata/atavar.h
diff -u src/sys/dev/ata/atavar.h:1.92.8.14 src/sys/dev/ata/atavar.h:1.92.8.15
--- src/sys/dev/ata/atavar.h:1.92.8.14 Wed Jun 21 22:40:43 2017
+++ src/sys/dev/ata/atavar.h Fri Jun 23 20:40:51 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: atavar.h,v 1.92.8.14 2017/06/21 22:40:43 jdolecek Exp $ */
+/* $NetBSD: atavar.h,v 1.92.8.15 2017/06/23 20:40:51 jdolecek Exp $ */
/*
* Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -299,7 +299,8 @@ struct ata_drive_datas {
#endif
/* Callbacks into the drive's driver. */
- void (*drv_done)(void *, struct ata_xfer *); /* transfer is done */
+ void (*drv_done)(device_t, struct ata_xfer *); /* xfer is done */
+ void (*drv_start)(device_t); /* start queue */
device_t drv_softc; /* ATA drives softc, if any */
struct ata_channel *chnl_softc; /* channel softc */
@@ -499,6 +500,7 @@ void ata_kill_active(struct ata_channel
void ata_reset_channel(struct ata_channel *, int);
void ata_channel_freeze(struct ata_channel *);
void ata_channel_thaw(struct ata_channel *);
+void ata_channel_start(struct ata_channel *, int);
int ata_addref(struct ata_channel *);
void ata_delref(struct ata_channel *);
Index: src/sys/dev/ata/wd.c
diff -u src/sys/dev/ata/wd.c:1.428.2.19 src/sys/dev/ata/wd.c:1.428.2.20
--- src/sys/dev/ata/wd.c:1.428.2.19 Tue Jun 20 20:58:22 2017
+++ src/sys/dev/ata/wd.c Fri Jun 23 20:40:51 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: wd.c,v 1.428.2.19 2017/06/20 20:58:22 jdolecek Exp $ */
+/* $NetBSD: wd.c,v 1.428.2.20 2017/06/23 20:40:51 jdolecek Exp $ */
/*
* Copyright (c) 1998, 2001 Manuel Bouyer. All rights reserved.
@@ -54,7 +54,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: wd.c,v 1.428.2.19 2017/06/20 20:58:22 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: wd.c,v 1.428.2.20 2017/06/23 20:40:51 jdolecek Exp $");
#include "opt_ata.h"
@@ -192,10 +192,10 @@ void wdioctlstrategy(struct buf *);
void wdgetdefaultlabel(struct wd_softc *, struct disklabel *);
void wdgetdisklabel(struct wd_softc *);
-void wdstart(struct wd_softc *);
+void wdstart(device_t);
void wdstart1(struct wd_softc *, struct buf *, struct ata_xfer *);
void wdrestart(void *);
-void wddone(void *, struct ata_xfer *);
+void wddone(device_t, struct ata_xfer *);
static void wd_params_to_properties(struct wd_softc *);
int wd_get_params(struct wd_softc *, uint8_t, struct ataparams *);
int wd_flushcache(struct wd_softc *, int);
@@ -310,6 +310,7 @@ wdattach(device_t parent, device_t self,
wd->drvp = adev->adev_drv_data;
wd->drvp->drv_openings = 1;
+ wd->drvp->drv_start = wdstart;
wd->drvp->drv_done = wddone;
wd->drvp->drv_softc = wd->sc_dev; /* done in atabusconfig_thread()
but too late */
@@ -518,6 +519,7 @@ wddetach(device_t self, int flags)
/* Unhook the entropy source. */
rnd_detach_source(&sc->rnd_source);
+ mutex_destroy(&sc->sc_lock);
callout_destroy(&sc->sc_restart_ch);
sc->drvp->drive_type = ATA_DRIVET_NONE; /* no drive any more here */
@@ -620,7 +622,8 @@ wdstrategy(struct buf *bp)
bufq_put(wd->sc_q, bp);
mutex_exit(&wd->sc_lock);
- wdstart(wd);
+ /* Try to queue on the current drive only */
+ wdstart(wd->sc_dev);
return;
done:
/* Toss transfer; we're done early. */
@@ -632,8 +635,9 @@ done:
* Queue a drive for I/O.
*/
void
-wdstart(struct wd_softc *wd)
+wdstart(device_t self)
{
+ struct wd_softc *wd = device_private(self);
struct buf *bp;
struct ata_xfer *xfer;
@@ -645,11 +649,19 @@ wdstart(struct wd_softc *wd)
mutex_enter(&wd->sc_lock);
+ /*
+ * Do not queue any transfers until flush is finished, so that
+ * once flush is pending, it will get handled as soon as xfer
+ * is available.
+ */
+ if (ISSET(wd->sc_flags, WDF_FLUSH_PEND))
+ goto out;
+
while (bufq_peek(wd->sc_q) != NULL) {
/* First try to get command */
xfer = ata_get_xfer_ext(wd->drvp->chnl_softc, false,
wd->drvp->drv_openings);
- if (xfer == NULL)
+ if (xfer == NULL)
break;
/* There is got to be a buf for us */
@@ -660,6 +672,7 @@ wdstart(struct wd_softc *wd)
wdstart1(wd, bp, xfer);
}
+out:
mutex_exit(&wd->sc_lock);
}
@@ -739,9 +752,9 @@ wdstart1(struct wd_softc *wd, struct buf
}
void
-wddone(void *v, struct ata_xfer *xfer)
+wddone(device_t self, struct ata_xfer *xfer)
{
- struct wd_softc *wd = device_private(v);
+ struct wd_softc *wd = device_private(self);
const char *errmsg;
int do_perror = 0;
struct buf *bp;
@@ -854,7 +867,7 @@ noerror: if ((xfer->c_bio.flags & ATA_CO
ata_free_xfer(wd->drvp->chnl_softc, xfer);
mutex_exit(&wd->sc_lock);
biodone(bp);
- wdstart(wd);
+ ata_channel_start(wd->drvp->chnl_softc, wd->drvp->drive);
}
void
@@ -1861,6 +1874,7 @@ wd_setcache(struct wd_softc *wd, int bit
out:
ata_free_xfer(wd->drvp->chnl_softc, xfer);
+ ata_channel_start(wd->drvp->chnl_softc, wd->drvp->drive);
return error;
}
@@ -1903,6 +1917,7 @@ wd_standby(struct wd_softc *wd, int flag
out:
ata_free_xfer(wd->drvp->chnl_softc, xfer);
+ ata_channel_start(wd->drvp->chnl_softc, wd->drvp->drive);
return error;
}
@@ -1921,9 +1936,20 @@ wd_flushcache(struct wd_softc *wd, int f
wd->sc_params.atap_cmd_set2 == 0xffff))
return ENODEV;
+ mutex_enter(&wd->sc_lock);
+ SET(wd->sc_flags, WDF_FLUSH_PEND);
+ mutex_exit(&wd->sc_lock);
+
xfer = ata_get_xfer(wd->drvp->chnl_softc);
- if (xfer == NULL)
- return EINTR;
+
+ mutex_enter(&wd->sc_lock);
+ CLR(wd->sc_flags, WDF_FLUSH_PEND);
+ mutex_exit(&wd->sc_lock);
+
+ if (xfer == NULL) {
+ error = EINTR;
+ goto out;
+ }
if ((wd->sc_params.atap_cmd2_en & ATA_CMD2_LBA48) != 0 &&
(wd->sc_params.atap_cmd2_en & ATA_CMD2_FCE) != 0) {
@@ -1939,13 +1965,13 @@ wd_flushcache(struct wd_softc *wd, int f
aprint_error_dev(wd->sc_dev,
"flush cache command didn't complete\n");
error = EIO;
- goto out;
+ goto out_xfer;
}
if (xfer->c_ata_c.flags & AT_ERROR) {
if (xfer->c_ata_c.r_error == WDCE_ABRT) {
/* command not supported */
error = ENODEV;
- goto out;
+ goto out_xfer;
}
}
if (xfer->c_ata_c.flags & (AT_ERROR | AT_TIMEOU | AT_DF)) {
@@ -1954,12 +1980,17 @@ wd_flushcache(struct wd_softc *wd, int f
aprint_error_dev(wd->sc_dev, "wd_flushcache: status=%s\n",
sbuf);
error = EIO;
- goto out;
+ goto out_xfer;
}
error = 0;
-out:
+out_xfer:
ata_free_xfer(wd->drvp->chnl_softc, xfer);
+
+out:
+ /* kick queue processing blocked while waiting for flush xfer */
+ ata_channel_start(wd->drvp->chnl_softc, wd->drvp->drive);
+
return error;
}
@@ -2023,6 +2054,7 @@ wd_trim(struct wd_softc *wd, int part, d
out:
ata_free_xfer(wd->drvp->chnl_softc, xfer);
+ ata_channel_start(wd->drvp->chnl_softc, wd->drvp->drive);
return error;
}
@@ -2233,6 +2265,8 @@ wdioctlstrategy(struct buf *bp)
out:
ata_free_xfer(wi->wi_softc->drvp->chnl_softc, xfer);
+ ata_channel_start(wi->wi_softc->drvp->chnl_softc,
+ wi->wi_softc->drvp->drive);
out2:
bp->b_error = error;
if (error)
Index: src/sys/dev/ata/wdvar.h
diff -u src/sys/dev/ata/wdvar.h:1.43.4.5 src/sys/dev/ata/wdvar.h:1.43.4.6
--- src/sys/dev/ata/wdvar.h:1.43.4.5 Sun Apr 23 01:21:04 2017
+++ src/sys/dev/ata/wdvar.h Fri Jun 23 20:40:51 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: wdvar.h,v 1.43.4.5 2017/04/23 01:21:04 jakllsch Exp $ */
+/* $NetBSD: wdvar.h,v 1.43.4.6 2017/06/23 20:40:51 jdolecek Exp $ */
/*
* Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -58,6 +58,7 @@ struct wd_softc {
#define WDF_LBA 0x040 /* using LBA mode */
#define WDF_KLABEL 0x080 /* retain label after 'full' close */
#define WDF_LBA48 0x100 /* using 48-bit LBA mode */
+#define WDF_FLUSH_PEND 0x200 /* cache flush waits for free xfer */
uint64_t sc_capacity; /* full capacity of the device */
uint64_t sc_capacity512; /* ... in DEV_BSIZE blocks */
uint32_t sc_capacity28; /* capacity accessible with LBA28 commands */
