Module Name: src
Committed By: jdolecek
Date: Sat Jul 29 12:58:30 UTC 2017
Modified Files:
src/sys/dev/ata [jdolecek-ncq]: ata.c atavar.h
Log Message:
reserve the highest slot for error recovery, and also have ata_channel
include space for the READ LOG EXT sector, so that it's not necessary
to allocate memory on the error handling path; now ata_read_log_ext_ncq()
will never fail due to resource shortage
To generate a diff of this commit:
cvs rdiff -u -r1.132.8.21 -r1.132.8.22 src/sys/dev/ata/ata.c
cvs rdiff -u -r1.92.8.19 -r1.92.8.20 src/sys/dev/ata/atavar.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/ata.c
diff -u src/sys/dev/ata/ata.c:1.132.8.21 src/sys/dev/ata/ata.c:1.132.8.22
--- src/sys/dev/ata/ata.c:1.132.8.21 Sat Jul 22 22:02:21 2017
+++ src/sys/dev/ata/ata.c Sat Jul 29 12:58:29 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: ata.c,v 1.132.8.21 2017/07/22 22:02:21 jdolecek Exp $ */
+/* $NetBSD: ata.c,v 1.132.8.22 2017/07/29 12:58:29 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.21 2017/07/22 22:02:21 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.132.8.22 2017/07/29 12:58:29 jdolecek Exp $");
#include "opt_ata.h"
@@ -191,7 +191,7 @@ ata_queue_reset(struct ata_queue *chq)
chq->queue_freeze = 0;
chq->queue_active = 0;
chq->active_xfers_used = 0;
- chq->queue_xfers_avail = (1 << chq->queue_openings) - 1;
+ chq->queue_xfers_avail = __BIT(chq->queue_openings) - 1;
}
struct ata_xfer *
@@ -202,7 +202,8 @@ ata_queue_hwslot_to_xfer(struct ata_chan
mutex_enter(&chp->ch_lock);
- KASSERT(hwslot < chq->queue_openings);
+ KASSERTMSG(hwslot < chq->queue_openings, "hwslot %d > openings %d",
+ hwslot, chq->queue_openings);
KASSERT((chq->active_xfers_used & __BIT(hwslot)) != 0);
/* Usually the first entry will be the one */
@@ -1048,20 +1049,9 @@ ata_read_log_ext_ncq(struct ata_drive_da
(drvp->drive_flags & ATA_DRIVE_NCQ) == 0)
return EOPNOTSUPP;
- xfer = ata_get_xfer_ext(chp, false, 0);
- if (xfer == NULL) {
- ATADEBUG_PRINT(("%s: no xfer\n", __func__),
- DEBUG_FUNCS|DEBUG_XFERS);
- return EAGAIN;
- }
+ xfer = ata_get_xfer_ext(chp, C_RECOVERY, 0);
- tb = malloc(DEV_BSIZE, M_DEVBUF, M_NOWAIT);
- if (tb == NULL) {
- ATADEBUG_PRINT(("%s: memory allocation failed\n", __func__),
- DEBUG_FUNCS|DEBUG_XFERS);
- rv = EAGAIN;
- goto out;
- }
+ tb = chp->ch_recovery;
memset(tb, 0, DEV_BSIZE);
/*
@@ -1070,14 +1060,14 @@ ata_read_log_ext_ncq(struct ata_drive_da
* and to make this a little faster. Realistically, it
* should not matter.
*/
- xfer->c_flags |= C_IMMEDIATE;
+ xfer->c_flags |= C_RECOVERY;
xfer->c_ata_c.r_command = WDCC_READ_LOG_EXT;
xfer->c_ata_c.r_lba = page = WDCC_LOG_PAGE_NCQ;
xfer->c_ata_c.r_st_bmask = WDCS_DRDY;
xfer->c_ata_c.r_st_pmask = WDCS_DRDY;
xfer->c_ata_c.r_count = 1;
xfer->c_ata_c.r_device = WDSD_LBA;
- xfer->c_ata_c.flags = AT_READ | AT_LBA | AT_LBA48 | flags;
+ xfer->c_ata_c.flags = AT_READ | AT_LBA | flags;
xfer->c_ata_c.timeout = 1000; /* 1s */
xfer->c_ata_c.data = tb;
xfer->c_ata_c.bcount = DEV_BSIZE;
@@ -1085,11 +1075,11 @@ ata_read_log_ext_ncq(struct ata_drive_da
if ((*atac->atac_bustype_ata->ata_exec_command)(drvp,
xfer) != ATACMD_COMPLETE) {
rv = EAGAIN;
- goto out2;
+ goto out;
}
if (xfer->c_ata_c.flags & (AT_ERROR | AT_TIMEOU | AT_DF)) {
rv = EINVAL;
- goto out2;
+ goto out;
}
cksum = 0;
@@ -1100,23 +1090,26 @@ ata_read_log_ext_ncq(struct ata_drive_da
"invalid checksum %x for READ LOG EXT page %x\n",
cksum, page);
rv = EINVAL;
- goto out2;
+ goto out;
}
if (tb[0] & WDCC_LOG_NQ) {
/* not queued command */
rv = EOPNOTSUPP;
- goto out2;
+ goto out;
}
*slot = tb[0] & 0x1f;
*status = tb[2];
*err = tb[3];
+ KASSERTMSG((*status & WDCS_ERR),
+ "%s: non-error command slot %d reported by READ LOG EXT page %x: "
+ "err %x status %x\n",
+ device_xname(drvp->drv_softc), *slot, page, *err, *status);
+
rv = 0;
-out2:
- free(tb, DEV_BSIZE);
out:
ata_free_xfer(chp, xfer);
return rv;
@@ -1180,12 +1173,15 @@ ata_exec_xfer(struct ata_channel *chp, s
mutex_enter(&chp->ch_lock);
- /* insert at the end of command list unless specially requested */
- if (xfer->c_flags & C_IMMEDIATE)
- TAILQ_INSERT_HEAD(&chp->ch_queue->queue_xfer, xfer,
+ /*
+ * Standard commands are added to the end of command list, but
+ * recovery commands must be run immediatelly.
+ */
+ if ((xfer->c_flags & C_RECOVERY) == 0)
+ TAILQ_INSERT_TAIL(&chp->ch_queue->queue_xfer, xfer,
c_xferchain);
else
- TAILQ_INSERT_TAIL(&chp->ch_queue->queue_xfer, xfer,
+ TAILQ_INSERT_HEAD(&chp->ch_queue->queue_xfer, xfer,
c_xferchain);
ATADEBUG_PRINT(("atastart from ata_exec_xfer, flags 0x%x\n",
chp->ch_flags), DEBUG_XFERS);
@@ -1255,7 +1251,7 @@ atastart(struct ata_channel *chp)
if ((xfer = TAILQ_FIRST(&chp->ch_queue->queue_xfer)) == NULL)
goto out;
- immediate = ISSET(xfer->c_flags, C_IMMEDIATE);
+ immediate = ISSET(xfer->c_flags, C_RECOVERY);
/* is the queue frozen? */
if (__predict_false(!immediate && chq->queue_freeze > 0)) {
@@ -1330,28 +1326,56 @@ out:
/*
* Does it's own locking, does not require splbio().
- * wait - whether to block waiting for free xfer
+ * flags - whether to block waiting for free xfer
* openings - limit of openings supported by device, <= 0 means tag not
* relevant, and any available xfer can be returned
*/
struct ata_xfer *
-ata_get_xfer_ext(struct ata_channel *chp, bool wait, int8_t openings)
+ata_get_xfer_ext(struct ata_channel *chp, int flags, uint8_t openings)
{
struct ata_queue *chq = chp->ch_queue;
struct ata_xfer *xfer = NULL;
uint32_t avail, slot, mask;
int error;
+ ATADEBUG_PRINT(("%s: channel %d flags %x openings %d\n",
+ __func__, chp->ch_channel, flags, openings),
+ DEBUG_XFERS);
+
mutex_enter(&chp->ch_lock);
-retry:
- mask = (openings > 0)
- ? (__BIT(MIN(ATA_MAX_OPENINGS, openings)) - 1)
- : chq->queue_xfers_avail;
+ /*
+ * When openings is just 1, can't reserve anything for
+ * recovery. KASSERT() here is to catch code which naively
+ * relies on C_RECOVERY to work under this condition.
+ */
+ KASSERT((flags & C_RECOVERY) == 0 || chq->queue_openings > 1);
+
+ if (flags & C_RECOVERY) {
+ mask = UINT32_MAX;
+ } else {
+ if (openings <= 0 || openings > chq->queue_openings)
+ openings = chq->queue_openings;
+ if (openings > 1) {
+ mask = __BIT(openings - 1) - 1;
+ } else {
+ mask = UINT32_MAX;
+ }
+ }
+
+retry:
avail = ffs32(chq->queue_xfers_avail & mask);
if (avail == 0) {
- if (wait) {
+ /*
+ * Catch code which tries to get another recovery xfer while
+ * already holding one (wrong recursion).
+ */
+ KASSERTMSG((flags & C_RECOVERY) == 0,
+ "recovery xfer busy openings %d mask %x avail %x",
+ openings, mask, chq->queue_xfers_avail);
+
+ if (flags & C_WAIT) {
chq->queue_flags |= QF_NEED_XFER;
error = cv_wait_sig(&chq->queue_busy, &chp->ch_lock);
if (error == 0)
@@ -1786,7 +1810,7 @@ ata_print_modes(struct ata_channel *chp)
if (drvp->drive_flags & ATA_DRIVE_NCQ) {
aprint_verbose(", NCQ (%d tags)%s",
- chp->ch_queue->queue_openings,
+ ATA_REAL_OPENINGS(chp->ch_queue->queue_openings),
(drvp->drive_flags & ATA_DRIVE_NCQ_PRIO)
? " w/PRIO" : "");
} else if (drvp->drive_flags & ATA_DRIVE_WFUA)
Index: src/sys/dev/ata/atavar.h
diff -u src/sys/dev/ata/atavar.h:1.92.8.19 src/sys/dev/ata/atavar.h:1.92.8.20
--- src/sys/dev/ata/atavar.h:1.92.8.19 Sat Jul 22 22:02:21 2017
+++ src/sys/dev/ata/atavar.h Sat Jul 29 12:58:29 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: atavar.h,v 1.92.8.19 2017/07/22 22:02:21 jdolecek Exp $ */
+/* $NetBSD: atavar.h,v 1.92.8.20 2017/07/29 12:58:29 jdolecek Exp $ */
/*
* Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -184,7 +184,7 @@ struct ata_xfer {
#define C_FREE 0x0040 /* call ata_free_xfer() asap */
#define C_PIOBM 0x0080 /* command uses busmastering PIO */
#define C_NCQ 0x0100 /* command is queued */
-#define C_IMMEDIATE 0x0200 /* execute command without queuing */
+#define C_RECOVERY 0x0200 /* executed as part of recovery */
#define C_WAITTIMO 0x0400 /* race vs. timeout */
#define C_CHAOS 0x0800 /* forced error xfer */
@@ -197,9 +197,10 @@ struct ata_xfer {
/*
* While hw supports up to 32 tags, in practice we must never
* allow 32 active commands, since that would signal same as
- * channel error. So just limit this to 31.
+ * channel error. We use slot 32 only for error recovery if available.
*/
-#define ATA_MAX_OPENINGS 31
+#define ATA_MAX_OPENINGS 32
+#define ATA_REAL_OPENINGS(op) ((op) > 1 ? (op) - 1 : 1)
/* Per-channel queue of ata_xfers */
#ifndef ATABUS_PRIVATE
@@ -210,7 +211,7 @@ struct ata_queue {
#define QF_IDLE_WAIT 0x01 /* someone wants the controller idle */
#define QF_NEED_XFER 0x02 /* someone wants xfer */
int8_t queue_active; /* number of active transfers */
- int8_t queue_openings; /* max number of active xfers */
+ uint8_t queue_openings; /* max number of active xfers */
TAILQ_HEAD(, ata_xfer) queue_xfer; /* queue of pending commands */
int queue_freeze; /* freeze count for the queue */
kcondvar_t queue_busy; /* c: waiting of xfer */
@@ -424,6 +425,9 @@ struct ata_channel {
/* Number of sata PMP ports, if any */
int ch_satapmp_nports;
+
+ /* Recovery buffer */
+ uint8_t ch_recovery[DEV_BSIZE];
};
/*
@@ -505,8 +509,8 @@ int ata_read_log_ext_ncq(struct ata_driv
#define CMD_ERR 1
#define CMD_AGAIN 2
-struct ata_xfer *ata_get_xfer_ext(struct ata_channel *, bool, int8_t);
-#define ata_get_xfer(chp) ata_get_xfer_ext((chp), true, 0);
+struct ata_xfer *ata_get_xfer_ext(struct ata_channel *, int, uint8_t);
+#define ata_get_xfer(chp) ata_get_xfer_ext((chp), C_WAIT, 0);
void ata_free_xfer(struct ata_channel *, struct ata_xfer *);
void ata_deactivate_xfer(struct ata_channel *, struct ata_xfer *);
void ata_exec_xfer(struct ata_channel *, struct ata_xfer *);
