Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/dev fix deadlock in wdcwait() when xfer timeout happens ...
details: https://anonhg.NetBSD.org/src/rev/8c8c7b02da8b
branches: trunk
changeset: 1008886:8c8c7b02da8b
user: jdolecek <jdolecek%NetBSD.org@localhost>
date: Sat Apr 04 21:36:15 2020 +0000
description:
fix deadlock in wdcwait() when xfer timeout happens while the atabus
thread sleeps in wdcwait() - check current lwp rather than relying
on global ATACH_TH_RUN channel flag
should fix the hang part of the problem reported in
http://mail-index.netbsd.org/netbsd-users/2020/03/12/msg024249.html
thanks to Paul Ripke for providing extensive debugging info
diffstat:
sys/dev/ata/ata.c | 29 +++++++++++++++--------------
sys/dev/ata/ata_wdc.c | 9 ++++-----
sys/dev/ata/atavar.h | 4 ++--
sys/dev/ic/mvsata.c | 16 ++++++++--------
sys/dev/ic/wdc.c | 7 +++----
sys/dev/scsipi/atapi_wdc.c | 9 ++++-----
6 files changed, 36 insertions(+), 38 deletions(-)
diffs (261 lines):
diff -r 18e02f311da5 -r 8c8c7b02da8b sys/dev/ata/ata.c
--- a/sys/dev/ata/ata.c Sat Apr 04 21:29:54 2020 +0000
+++ b/sys/dev/ata/ata.c Sat Apr 04 21:36:15 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ata.c,v 1.153 2019/10/21 18:58:57 christos Exp $ */
+/* $NetBSD: ata.c,v 1.154 2020/04/04 21:36:15 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.153 2019/10/21 18:58:57 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.154 2020/04/04 21:36:15 jdolecek Exp $");
#include "opt_ata.h"
@@ -44,6 +44,7 @@
#include <sys/bus.h>
#include <sys/once.h>
#include <sys/bitops.h>
+#include <sys/cpu.h>
#define ATABUS_PRIVATE
@@ -229,9 +230,6 @@
int i, error;
/* we are in the atabus's thread context */
- ata_channel_lock(chp);
- chp->ch_flags |= ATACH_TH_RUN;
- ata_channel_unlock(chp);
/*
* Probe for the drives attached to controller, unless a PMP
@@ -247,11 +245,6 @@
DEBUG_PROBE);
}
- /* next operations will occurs in a separate thread */
- ata_channel_lock(chp);
- chp->ch_flags &= ~ATACH_TH_RUN;
- ata_channel_unlock(chp);
-
/* Make sure the devices probe in atabus order to avoid jitter. */
mutex_enter(&atabus_qlock);
for (;;) {
@@ -264,6 +257,8 @@
ata_channel_lock(chp);
+ KASSERT(ata_is_thread_run(chp));
+
/* If no drives, abort here */
if (chp->ch_drive == NULL)
goto out;
@@ -444,7 +439,7 @@
int i, rv;
ata_channel_lock(chp);
- chp->ch_flags |= ATACH_TH_RUN;
+ KASSERT(ata_is_thread_run(chp));
/*
* Probe the drives. Reset type to indicate to controllers
@@ -466,9 +461,7 @@
if ((chp->ch_flags & (ATACH_TH_RESET | ATACH_TH_DRIVE_RESET
| ATACH_TH_RECOVERY | ATACH_SHUTDOWN)) == 0 &&
(chq->queue_active == 0 || chq->queue_freeze == 0)) {
- chp->ch_flags &= ~ATACH_TH_RUN;
cv_wait(&chp->ch_thr_idle, &chp->ch_lock);
- chp->ch_flags |= ATACH_TH_RUN;
}
if (chp->ch_flags & ATACH_SHUTDOWN) {
break;
@@ -547,6 +540,14 @@
kthread_exit(0);
}
+bool
+ata_is_thread_run(struct ata_channel *chp)
+{
+ KASSERT(mutex_owned(&chp->ch_lock));
+
+ return (chp->ch_thread == curlwp && !cpu_intr_p());
+}
+
static void
ata_thread_wake_locked(struct ata_channel *chp)
{
@@ -1896,7 +1897,7 @@
*/
if (atac->atac_set_modes)
/*
- * It's OK to pool here, it's fast enough
+ * It's OK to poll here, it's fast enough
* to not bother waiting for interrupt
*/
if (ata_set_mode(drvp, 0x08 | (i + 3),
diff -r 18e02f311da5 -r 8c8c7b02da8b sys/dev/ata/ata_wdc.c
--- a/sys/dev/ata/ata_wdc.c Sat Apr 04 21:29:54 2020 +0000
+++ b/sys/dev/ata/ata_wdc.c Sat Apr 04 21:36:15 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ata_wdc.c,v 1.113 2018/11/12 18:51:01 jdolecek Exp $ */
+/* $NetBSD: ata_wdc.c,v 1.114 2020/04/04 21:36:15 jdolecek 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.113 2018/11/12 18:51:01 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.114 2020/04/04 21:36:15 jdolecek Exp $");
#include "opt_ata.h"
#include "opt_wdc.h"
@@ -206,10 +206,9 @@
* that we never get to this point if that's the case.
*/
/* If it's not a polled command, we need the kernel thread */
- if ((xfer->c_flags & C_POLL) == 0 &&
- (chp->ch_flags & ATACH_TH_RUN) == 0) {
+ if ((xfer->c_flags & C_POLL) == 0 && !ata_is_thread_run(chp))
return ATASTART_TH;
- }
+
/*
* disable interrupts, all commands here should be quick
* enough to be able to poll, and we don't go here that often
diff -r 18e02f311da5 -r 8c8c7b02da8b sys/dev/ata/atavar.h
--- a/sys/dev/ata/atavar.h Sat Apr 04 21:29:54 2020 +0000
+++ b/sys/dev/ata/atavar.h Sat Apr 04 21:36:15 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: atavar.h,v 1.103 2019/04/05 21:31:44 bouyer Exp $ */
+/* $NetBSD: atavar.h,v 1.104 2020/04/04 21:36:15 jdolecek Exp $ */
/*
* Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -406,7 +406,6 @@
#define ATACH_DMA_WAIT 0x20 /* controller is waiting for DMA */
#define ATACH_PIOBM_WAIT 0x40 /* controller is waiting for busmastering PIO */
#define ATACH_DISABLED 0x80 /* channel is disabled */
-#define ATACH_TH_RUN 0x100 /* the kernel thread is working */
#define ATACH_TH_RESET 0x200 /* someone ask the thread to reset */
#define ATACH_TH_RESCAN 0x400 /* rescan requested */
#define ATACH_NCQ 0x800 /* channel executing NCQ commands */
@@ -549,6 +548,7 @@
void ata_kill_pending(struct ata_drive_datas *);
void ata_kill_active(struct ata_channel *, int, int);
void ata_thread_run(struct ata_channel *, int, int, int);
+bool ata_is_thread_run(struct ata_channel *);
void ata_channel_freeze(struct ata_channel *);
void ata_channel_thaw_locked(struct ata_channel *);
void ata_channel_lock(struct ata_channel *);
diff -r 18e02f311da5 -r 8c8c7b02da8b sys/dev/ic/mvsata.c
--- a/sys/dev/ic/mvsata.c Sat Apr 04 21:29:54 2020 +0000
+++ b/sys/dev/ic/mvsata.c Sat Apr 04 21:36:15 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: mvsata.c,v 1.54 2020/03/22 16:46:30 macallan Exp $ */
+/* $NetBSD: mvsata.c,v 1.55 2020/04/04 21:36:15 jdolecek 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.54 2020/03/22 16:46:30 macallan Exp $");
+__KERNEL_RCSID(0, "$NetBSD: mvsata.c,v 1.55 2020/04/04 21:36:15 jdolecek Exp $");
#include "opt_mvsata.h"
@@ -1165,10 +1165,10 @@
* If it's not a polled command, we need the kernel
* thread
*/
- if ((xfer->c_flags & C_POLL) == 0 &&
- (chp->ch_flags & ATACH_TH_RUN) == 0) {
+ if ((xfer->c_flags & C_POLL) == 0
+ && !ata_is_thread_run(chp))
return ATASTART_TH;
- }
+
if (mvsata_bio_ready(mvport, ata_bio, xfer->c_drive,
(xfer->c_flags & C_POLL) ? AT_POLL : 0) != 0) {
return ATASTART_ABORT;
@@ -2052,10 +2052,10 @@
/* Do control operations specially. */
if (__predict_false(drvp->state < READY)) {
/* If it's not a polled command, we need the kernel thread */
- if ((sc_xfer->xs_control & XS_CTL_POLL) == 0 &&
- (chp->ch_flags & ATACH_TH_RUN) == 0) {
+ if ((sc_xfer->xs_control & XS_CTL_POLL) == 0
+ && !ata_is_thread_run(chp))
return ATASTART_TH;
- }
+
/*
* disable interrupts, all commands here should be quick
* enough to be able to poll, and we don't go here that often
diff -r 18e02f311da5 -r 8c8c7b02da8b sys/dev/ic/wdc.c
--- a/sys/dev/ic/wdc.c Sat Apr 04 21:29:54 2020 +0000
+++ b/sys/dev/ic/wdc.c Sat Apr 04 21:36:15 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: wdc.c,v 1.297 2020/02/10 20:11:48 jdolecek Exp $ */
+/* $NetBSD: wdc.c,v 1.298 2020/04/04 21:36:15 jdolecek Exp $ */
/*
* Copyright (c) 1998, 2001, 2003 Manuel Bouyer. All rights reserved.
@@ -58,7 +58,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: wdc.c,v 1.297 2020/02/10 20:11:48 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: wdc.c,v 1.298 2020/04/04 21:36:15 jdolecek Exp $");
#include "opt_ata.h"
#include "opt_wdc.h"
@@ -1278,8 +1278,7 @@
else {
error = __wdcwait(chp, mask, bits, WDCDELAY_POLL, tfd);
if (error != 0) {
- if ((chp->ch_flags & ATACH_TH_RUN) ||
- (flags & AT_WAIT)) {
+ if (ata_is_thread_run(chp) || (flags & AT_WAIT)) {
/*
* we're running in the channel thread
* or some userland thread context
diff -r 18e02f311da5 -r 8c8c7b02da8b sys/dev/scsipi/atapi_wdc.c
--- a/sys/dev/scsipi/atapi_wdc.c Sat Apr 04 21:29:54 2020 +0000
+++ b/sys/dev/scsipi/atapi_wdc.c Sat Apr 04 21:36:15 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: atapi_wdc.c,v 1.136 2020/02/19 16:04:39 riastradh Exp $ */
+/* $NetBSD: atapi_wdc.c,v 1.137 2020/04/04 21:36:15 jdolecek Exp $ */
/*
* Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -25,7 +25,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: atapi_wdc.c,v 1.136 2020/02/19 16:04:39 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: atapi_wdc.c,v 1.137 2020/04/04 21:36:15 jdolecek Exp $");
#ifndef ATADEBUG
#define ATADEBUG
@@ -501,10 +501,9 @@
/* Do control operations specially. */
if (__predict_false(drvp->state < READY)) {
/* If it's not a polled command, we need the kernel thread */
- if ((sc_xfer->xs_control & XS_CTL_POLL) == 0 &&
- (chp->ch_flags & ATACH_TH_RUN) == 0) {
+ if ((sc_xfer->xs_control & XS_CTL_POLL) == 0
+ && !ata_is_thread_run(chp))
return ATASTART_TH;
- }
/*
* disable interrupts, all commands here should be quick
* enough to be able to poll, and we don't go here that often
Home |
Main Index |
Thread Index |
Old Index