Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/dev PR kern/56403
details: https://anonhg.NetBSD.org/src/rev/e875a887d862
branches: trunk
changeset: 988239:e875a887d862
user: rin <rin%NetBSD.org@localhost>
date: Tue Oct 05 08:01:05 2021 +0000
description:
PR kern/56403
Fix kernel freeze for wdc(4) variants with ATAC_CAP_NOIRQ:
(1) Change ata_xfer_ops:c_poll from void to int function. When it returns
ATAPOLL_AGAIN, let ata_xfer_start() iterate itself again.
(2) Let wdc_ata_bio_poll() return ATAPOLL_AGAIN until ATA_ITSDONE is
achieved.
A similar change has been made for mvsata(4) (see mvsata_bio_poll()),
and no functional changes for other devices.
This is how the drivers worked before jdolecek-ncq branch was merged.
Note that this changes are less likely to cause infinite recursion:
(1) wdc_ata_bio_intr() called from wdc_ata_bio_poll() asserts ATA_ITSDONE
in its error handling paths via wdc_ata_bio_done().
(2) Return value from c_start (= wdc_ata_bio_start()) is checked in
ata_xfer_start().
Therefore, errors encountered in ata_xfer_ops:c_poll and c_start routines
terminate the recursion for wdc(4). The situation is similar for mvsata(4).
Still, there is a possibility where ata_xfer_start() takes long time to
finish a normal operation. This can result in a delayed response for lower
priority interrupts. But, I've never observed such a situation, even when
heavy thrashing takes place for swap partition in wd(4).
"Go ahead" by jdolecek@.
diffstat:
sys/dev/ata/ata.c | 11 +++++++----
sys/dev/ata/ata_wdc.c | 14 +++++++++-----
sys/dev/ata/atavar.h | 6 ++++--
sys/dev/ic/ahcisata_core.c | 20 ++++++++++++--------
sys/dev/ic/mvsata.c | 33 +++++++++++++++++++--------------
sys/dev/ic/siisata.c | 22 ++++++++++++++--------
sys/dev/ic/wdc.c | 9 +++++----
sys/dev/scsipi/atapi_wdc.c | 12 +++++++-----
8 files changed, 77 insertions(+), 50 deletions(-)
diffs (truncated from 530 to 300 lines):
diff -r 75538406bebf -r e875a887d862 sys/dev/ata/ata.c
--- a/sys/dev/ata/ata.c Tue Oct 05 07:05:51 2021 +0000
+++ b/sys/dev/ata/ata.c Tue Oct 05 08:01:05 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ata.c,v 1.163 2021/08/29 23:49:32 rin Exp $ */
+/* $NetBSD: ata.c,v 1.164 2021/10/05 08:01:05 rin 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.163 2021/08/29 23:49:32 rin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.164 2021/10/05 08:01:05 rin Exp $");
#include "opt_ata.h"
@@ -1231,10 +1231,11 @@
ata_xfer_start(struct ata_xfer *xfer)
{
struct ata_channel *chp = xfer->c_chp;
- int rv;
+ int rv, status;
KASSERT(mutex_owned(&chp->ch_lock));
+again:
rv = xfer->ops->c_start(chp, xfer);
switch (rv) {
case ATASTART_STARTED:
@@ -1248,8 +1249,10 @@
/* can happen even in thread context for some ATAPI devices */
ata_channel_unlock(chp);
KASSERT(xfer->ops != NULL && xfer->ops->c_poll != NULL);
- xfer->ops->c_poll(chp, xfer);
+ status = xfer->ops->c_poll(chp, xfer);
ata_channel_lock(chp);
+ if (status == ATAPOLL_AGAIN)
+ goto again;
break;
case ATASTART_ABORT:
ata_channel_unlock(chp);
diff -r 75538406bebf -r e875a887d862 sys/dev/ata/ata_wdc.c
--- a/sys/dev/ata/ata_wdc.c Tue Oct 05 07:05:51 2021 +0000
+++ b/sys/dev/ata/ata_wdc.c Tue Oct 05 08:01:05 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ata_wdc.c,v 1.119 2020/12/25 08:55:40 skrll Exp $ */
+/* $NetBSD: ata_wdc.c,v 1.120 2021/10/05 08:01:05 rin Exp $ */
/*
* Copyright (c) 1998, 2001, 2003 Manuel Bouyer.
@@ -54,7 +54,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.119 2020/12/25 08:55:40 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.120 2021/10/05 08:01:05 rin Exp $");
#include "opt_ata.h"
#include "opt_wdc.h"
@@ -105,7 +105,7 @@
static void wdc_ata_bio(struct ata_drive_datas*, struct ata_xfer *);
static int wdc_ata_bio_start(struct ata_channel *,struct ata_xfer *);
static int _wdc_ata_bio_start(struct ata_channel *,struct ata_xfer *);
-static void wdc_ata_bio_poll(struct ata_channel *,struct ata_xfer *);
+static int wdc_ata_bio_poll(struct ata_channel *,struct ata_xfer *);
static int wdc_ata_bio_intr(struct ata_channel *, struct ata_xfer *,
int);
static void wdc_ata_bio_kill_xfer(struct ata_channel *,
@@ -609,7 +609,7 @@
return ATASTART_ABORT;
}
-static void
+static int
wdc_ata_bio_poll(struct ata_channel *chp, struct ata_xfer *xfer)
{
/* Wait for at last 400ns for status bit to be valid */
@@ -621,6 +621,7 @@
}
#endif
wdc_ata_bio_intr(chp, xfer, 0);
+ return (xfer->c_bio.flags & ATA_ITSDONE) ? ATAPOLL_DONE : ATAPOLL_AGAIN;
}
static int
@@ -773,7 +774,10 @@
callout_stop(&chp->c_timo_callout);
ata_xfer_start(xfer);
} else {
- /* Let _wdc_ata_bio_start do the loop */
+ /*
+ * Let ata_xfer_start() do the loop;
+ * see wdc_ata_bio_poll().
+ */
}
ata_channel_unlock(chp);
return 1;
diff -r 75538406bebf -r e875a887d862 sys/dev/ata/atavar.h
--- a/sys/dev/ata/atavar.h Tue Oct 05 07:05:51 2021 +0000
+++ b/sys/dev/ata/atavar.h Tue Oct 05 08:01:05 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: atavar.h,v 1.108 2020/05/25 18:29:25 jdolecek Exp $ */
+/* $NetBSD: atavar.h,v 1.109 2021/10/05 08:01:05 rin Exp $ */
/*
* Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -178,7 +178,9 @@
#define ATASTART_TH 1 /* xfer needs to be run in thread */
#define ATASTART_POLL 2 /* xfer needs to be polled */
#define ATASTART_ABORT 3 /* error occurred, abort xfer */
- void (*c_poll)(struct ata_channel *, struct ata_xfer *);
+ int (*c_poll)(struct ata_channel *, struct ata_xfer *);
+#define ATAPOLL_DONE 0
+#define ATAPOLL_AGAIN 1
void (*c_abort)(struct ata_channel *, struct ata_xfer *);
int (*c_intr)(struct ata_channel *, struct ata_xfer *, int);
void (*c_kill_xfer)(struct ata_channel *, struct ata_xfer *, int);
diff -r 75538406bebf -r e875a887d862 sys/dev/ic/ahcisata_core.c
--- a/sys/dev/ic/ahcisata_core.c Tue Oct 05 07:05:51 2021 +0000
+++ b/sys/dev/ic/ahcisata_core.c Tue Oct 05 08:01:05 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ahcisata_core.c,v 1.101 2021/09/03 01:23:33 mrg Exp $ */
+/* $NetBSD: ahcisata_core.c,v 1.102 2021/10/05 08:01:05 rin Exp $ */
/*
* Copyright (c) 2006 Manuel Bouyer.
@@ -26,7 +26,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ahcisata_core.c,v 1.101 2021/09/03 01:23:33 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ahcisata_core.c,v 1.102 2021/10/05 08:01:05 rin Exp $");
#include <sys/types.h>
#include <sys/malloc.h>
@@ -69,13 +69,13 @@
static int ahci_cmd_start(struct ata_channel *, struct ata_xfer *);
static int ahci_cmd_complete(struct ata_channel *, struct ata_xfer *, int);
-static void ahci_cmd_poll(struct ata_channel *, struct ata_xfer *);
+static int ahci_cmd_poll(struct ata_channel *, struct ata_xfer *);
static void ahci_cmd_abort(struct ata_channel *, struct ata_xfer *);
static void ahci_cmd_done(struct ata_channel *, struct ata_xfer *);
static void ahci_cmd_done_end(struct ata_channel *, struct ata_xfer *);
static void ahci_cmd_kill_xfer(struct ata_channel *, struct ata_xfer *, int);
static int ahci_bio_start(struct ata_channel *, struct ata_xfer *);
-static void ahci_bio_poll(struct ata_channel *, struct ata_xfer *);
+static int ahci_bio_poll(struct ata_channel *, struct ata_xfer *);
static void ahci_bio_abort(struct ata_channel *, struct ata_xfer *);
static int ahci_bio_complete(struct ata_channel *, struct ata_xfer *, int);
static void ahci_bio_kill_xfer(struct ata_channel *, struct ata_xfer *, int) ;
@@ -93,7 +93,7 @@
static void ahci_atapi_scsipi_request(struct scsipi_channel *,
scsipi_adapter_req_t, void *);
static int ahci_atapi_start(struct ata_channel *, struct ata_xfer *);
-static void ahci_atapi_poll(struct ata_channel *, struct ata_xfer *);
+static int ahci_atapi_poll(struct ata_channel *, struct ata_xfer *);
static void ahci_atapi_abort(struct ata_channel *, struct ata_xfer *);
static int ahci_atapi_complete(struct ata_channel *, struct ata_xfer *, int);
static void ahci_atapi_kill_xfer(struct ata_channel *, struct ata_xfer *, int);
@@ -1208,7 +1208,7 @@
return ATASTART_POLL;
}
-static void
+static int
ahci_cmd_poll(struct ata_channel *chp, struct ata_xfer *xfer)
{
struct ahci_softc *sc = AHCI_CH2SC(chp);
@@ -1245,6 +1245,8 @@
}
/* reenable interrupts */
AHCI_WRITE(sc, AHCI_GHC, AHCI_READ(sc, AHCI_GHC) | AHCI_GHC_IE);
+
+ return ATAPOLL_DONE;
}
static void
@@ -1456,7 +1458,7 @@
return ATASTART_POLL;
}
-static void
+static int
ahci_bio_poll(struct ata_channel *chp, struct ata_xfer *xfer)
{
struct ahci_softc *sc = (struct ahci_softc *)chp->ch_atac;
@@ -1486,6 +1488,7 @@
}
/* reenable interrupts */
AHCI_WRITE(sc, AHCI_GHC, AHCI_READ(sc, AHCI_GHC) | AHCI_GHC_IE);
+ return ATAPOLL_DONE;
}
static void
@@ -1953,7 +1956,7 @@
return ATASTART_POLL;
}
-static void
+static int
ahci_atapi_poll(struct ata_channel *chp, struct ata_xfer *xfer)
{
struct ahci_softc *sc = (struct ahci_softc *)chp->ch_atac;
@@ -1983,6 +1986,7 @@
}
/* reenable interrupts */
AHCI_WRITE(sc, AHCI_GHC, AHCI_READ(sc, AHCI_GHC) | AHCI_GHC_IE);
+ return ATAPOLL_DONE;
}
static void
diff -r 75538406bebf -r e875a887d862 sys/dev/ic/mvsata.c
--- a/sys/dev/ic/mvsata.c Tue Oct 05 07:05:51 2021 +0000
+++ b/sys/dev/ic/mvsata.c Tue Oct 05 08:01:05 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: mvsata.c,v 1.60 2021/08/07 16:19:12 thorpej Exp $ */
+/* $NetBSD: mvsata.c,v 1.61 2021/10/05 08:01:05 rin Exp $ */
/*
* Copyright (c) 2008 KIYOHARA Takashi
* All rights reserved.
@@ -26,7 +26,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: mvsata.c,v 1.60 2021/08/07 16:19:12 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: mvsata.c,v 1.61 2021/10/05 08:01:05 rin Exp $");
#include "opt_mvsata.h"
@@ -143,14 +143,14 @@
#ifndef MVSATA_WITHOUTDMA
static int mvsata_bio_start(struct ata_channel *, struct ata_xfer *);
static int mvsata_bio_intr(struct ata_channel *, struct ata_xfer *, int);
-static void mvsata_bio_poll(struct ata_channel *, struct ata_xfer *);
+static int mvsata_bio_poll(struct ata_channel *, struct ata_xfer *);
static void mvsata_bio_kill_xfer(struct ata_channel *, struct ata_xfer *, int);
static void mvsata_bio_done(struct ata_channel *, struct ata_xfer *);
static int mvsata_bio_ready(struct mvsata_port *, struct ata_bio *, int,
int);
static int mvsata_wdc_cmd_start(struct ata_channel *, struct ata_xfer *);
static int mvsata_wdc_cmd_intr(struct ata_channel *, struct ata_xfer *, int);
-static void mvsata_wdc_cmd_poll(struct ata_channel *, struct ata_xfer *);
+static int mvsata_wdc_cmd_poll(struct ata_channel *, struct ata_xfer *);
static void mvsata_wdc_cmd_kill_xfer(struct ata_channel *, struct ata_xfer *,
int);
static void mvsata_wdc_cmd_done(struct ata_channel *, struct ata_xfer *);
@@ -158,7 +158,7 @@
#if NATAPIBUS > 0
static int mvsata_atapi_start(struct ata_channel *, struct ata_xfer *);
static int mvsata_atapi_intr(struct ata_channel *, struct ata_xfer *, int);
-static void mvsata_atapi_poll(struct ata_channel *, struct ata_xfer *);
+static int mvsata_atapi_poll(struct ata_channel *, struct ata_xfer *);
static void mvsata_atapi_kill_xfer(struct ata_channel *, struct ata_xfer *,
int);
static void mvsata_atapi_reset(struct ata_channel *, struct ata_xfer *);
@@ -1245,7 +1245,7 @@
return ATASTART_ABORT;
}
-static void
+static int
mvsata_bio_poll(struct ata_channel *chp, struct ata_xfer *xfer)
{
struct mvsata_port *mvport = (struct mvsata_port *)chp;
@@ -1259,10 +1259,9 @@
chp->ch_flags &= ~ATACH_DMA_WAIT;
}
- if ((xfer->c_bio.flags & ATA_ITSDONE) == 0) {
- KASSERT(xfer->c_flags & C_TIMEOU);
- mvsata_bio_intr(chp, xfer, 0);
- }
+ mvsata_bio_intr(chp, xfer, 0);
+
+ return (xfer->c_bio.flags & ATA_ITSDONE) ? ATAPOLL_DONE : ATAPOLL_AGAIN;
}
static int
@@ -1385,7 +1384,10 @@
/* Start the next operation */
ata_xfer_start(xfer);
} else {
- /* Let mvsata_bio_start do the loop */
+ /*
+ * Let ata_xfer_start() do the loop;
+ * see mvsata_bio_poll().
+ */
}
ata_channel_unlock(chp);
} else { /* Done with this transfer */
@@ -1680,7 +1682,7 @@
return ATASTART_POLL;
}
-static void
+static int
mvsata_wdc_cmd_poll(struct ata_channel *chp, struct ata_xfer *xfer)
Home |
Main Index |
Thread Index |
Old Index