Module Name:    src
Committed By:   jdolecek
Date:           Mon Nov 12 18:51:01 UTC 2018

Modified Files:
        src/sys/dev/ata: ata.c ata_wdc.c
        src/sys/dev/ic: mvsata.c
        src/sys/dev/scsipi: atapi_wdc.c

Log Message:
hold channel lock during whole ata_dmaerr()/ata_downgrade_mode() -
according to code inspection this is safe, none of the set_modes
hooks execute anything which would be taking the lock

adresses PR kern/53714 by Andreas Gustafsson


To generate a diff of this commit:
cvs rdiff -u -r1.145 -r1.146 src/sys/dev/ata/ata.c
cvs rdiff -u -r1.112 -r1.113 src/sys/dev/ata/ata_wdc.c
cvs rdiff -u -r1.44 -r1.45 src/sys/dev/ic/mvsata.c
cvs rdiff -u -r1.131 -r1.132 src/sys/dev/scsipi/atapi_wdc.c

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.145 src/sys/dev/ata/ata.c:1.146
--- src/sys/dev/ata/ata.c:1.145	Wed Oct 24 20:25:52 2018
+++ src/sys/dev/ata/ata.c	Mon Nov 12 18:51:01 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: ata.c,v 1.145 2018/10/24 20:25:52 jdolecek Exp $	*/
+/*	$NetBSD: ata.c,v 1.146 2018/11/12 18:51:01 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.145 2018/10/24 20:25:52 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.146 2018/11/12 18:51:01 jdolecek Exp $");
 
 #include "opt_ata.h"
 
@@ -952,6 +952,8 @@ out:
 void
 ata_dmaerr(struct ata_drive_datas *drvp, int flags)
 {
+	ata_channel_lock_owned(drvp->chnl_softc);
+
 	/*
 	 * Downgrade decision: if we get NERRS_MAX in NXFER.
 	 * We start with n_dmaerrs set to NERRS_MAX-1 so that the
@@ -1772,6 +1774,8 @@ ata_downgrade_mode(struct ata_drive_data
 	device_t drv_dev = drvp->drv_softc;
 	int cf_flags = device_cfdata(drv_dev)->cf_flags;
 
+	ata_channel_lock_owned(drvp->chnl_softc);
+
 	/* if drive or controller don't know its mode, we can't do much */
 	if ((drvp->drive_flags & ATA_DRIVE_MODE) == 0 ||
 	    (atac->atac_set_modes == NULL))

Index: src/sys/dev/ata/ata_wdc.c
diff -u src/sys/dev/ata/ata_wdc.c:1.112 src/sys/dev/ata/ata_wdc.c:1.113
--- src/sys/dev/ata/ata_wdc.c:1.112	Mon Oct 22 20:13:47 2018
+++ src/sys/dev/ata/ata_wdc.c	Mon Nov 12 18:51:01 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: ata_wdc.c,v 1.112 2018/10/22 20:13:47 jdolecek Exp $	*/
+/*	$NetBSD: ata_wdc.c,v 1.113 2018/11/12 18:51:01 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.112 2018/10/22 20:13:47 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.113 2018/11/12 18:51:01 jdolecek Exp $");
 
 #include "opt_ata.h"
 #include "opt_wdc.h"
@@ -729,10 +729,8 @@ wdc_ata_bio_intr(struct ata_channel *chp
 		if (drv_err != WDC_ATA_ERR)
 			goto end;
 		if (ata_bio->r_error & WDCE_CRC || ata_bio->error == ERR_DMA) {
-			ata_channel_unlock(chp);
 			ata_dmaerr(drvp,
 			    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
-			ata_channel_lock(chp);
 			goto err;
 		}
 	}

Index: src/sys/dev/ic/mvsata.c
diff -u src/sys/dev/ic/mvsata.c:1.44 src/sys/dev/ic/mvsata.c:1.45
--- src/sys/dev/ic/mvsata.c:1.44	Mon Oct 22 20:30:52 2018
+++ src/sys/dev/ic/mvsata.c	Mon Nov 12 18:51:01 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: mvsata.c,v 1.44 2018/10/22 20:30:52 jdolecek Exp $	*/
+/*	$NetBSD: mvsata.c,v 1.45 2018/11/12 18:51:01 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.44 2018/10/22 20:30:52 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: mvsata.c,v 1.45 2018/11/12 18:51:01 jdolecek Exp $");
 
 #include "opt_mvsata.h"
 
@@ -1360,9 +1360,9 @@ mvsata_bio_intr(struct ata_channel *chp,
 		if (ata_bio->error == NOERROR)
 			goto end;
 		if (ata_bio->error == ERR_DMA) {
-			ata_channel_unlock(chp);
 			ata_dmaerr(drvp,
 			    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
+			ata_channel_unlock(chp);
 			goto err;
 		}
 	}
@@ -2275,11 +2275,11 @@ mvsata_atapi_intr(struct ata_channel *ch
 		aprint_error_dev(atac->atac_dev,
 		    "channel %d: device timeout, c_bcount=%d, c_skip=%d\n",
 		    chp->ch_channel, xfer->c_bcount, xfer->c_skip);
-		ata_channel_unlock(chp);
 		if (xfer->c_flags & C_DMA)
 			ata_dmaerr(drvp,
 			    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
 		sc_xfer->error = XS_TIMEOUT;
+		ata_channel_unlock(chp);
 		mvsata_atapi_reset(chp, xfer);
 		return 1;
 	}
@@ -2289,9 +2289,9 @@ mvsata_atapi_intr(struct ata_channel *ch
 	 * and reset device.
 	 */
 	if ((xfer->c_flags & C_TIMEOU) && (xfer->c_flags & C_DMA)) {
-		ata_channel_unlock(chp);
 		ata_dmaerr(drvp, (xfer->c_flags & C_POLL) ? AT_POLL : 0);
 		sc_xfer->error = XS_RESET;
+		ata_channel_unlock(chp);
 		mvsata_atapi_reset(chp, xfer);
 		return (1);
 	}
@@ -2351,11 +2351,11 @@ again:
 			aprint_error_dev(atac->atac_dev,
 			    "channel %d drive %d: bad data phase DATAOUT\n",
 			    chp->ch_channel, xfer->c_drive);
-			ata_channel_unlock(chp);
 			if (xfer->c_flags & C_DMA)
 				ata_dmaerr(drvp,
 				    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
 			sc_xfer->error = XS_TIMEOUT;
+			ata_channel_unlock(chp);
 			mvsata_atapi_reset(chp, xfer);
 			return 1;
 		}
@@ -2387,10 +2387,10 @@ again:
 			aprint_error_dev(atac->atac_dev,
 			    "channel %d drive %d: bad data phase DATAIN\n",
 			    chp->ch_channel, xfer->c_drive);
-			ata_channel_unlock(chp);
 			if (xfer->c_flags & C_DMA)
 				ata_dmaerr(drvp,
 				    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
+			ata_channel_unlock(chp);
 			sc_xfer->error = XS_TIMEOUT;
 			mvsata_atapi_reset(chp, xfer);
 			return 1;
@@ -2441,11 +2441,11 @@ again:
 			sc_xfer->error = XS_SHORTSENSE;
 			sc_xfer->sense.atapi_sense = ATACH_ERR(tfd);
 		} else {
-			ata_channel_unlock(chp);
 			if (xfer->c_flags & C_DMA)
 				ata_dmaerr(drvp,
 				    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
 			sc_xfer->error = XS_RESET;
+			ata_channel_unlock(chp);
 			mvsata_atapi_reset(chp, xfer);
 			return (1);
 		}
@@ -2580,10 +2580,10 @@ mvsata_atapi_phase_complete(struct ata_x
 			sc_xfer->status = SCSI_CHECK;
 		} else
 		    if (wdc->dma_status & (WDC_DMAST_NOIRQ | WDC_DMAST_ERR)) {
-			ata_channel_unlock(chp);
 			ata_dmaerr(drvp,
 			    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
 			sc_xfer->error = XS_RESET;
+			ata_channel_unlock(chp);
 			mvsata_atapi_reset(chp, xfer);
 			return;
 		}

Index: src/sys/dev/scsipi/atapi_wdc.c
diff -u src/sys/dev/scsipi/atapi_wdc.c:1.131 src/sys/dev/scsipi/atapi_wdc.c:1.132
--- src/sys/dev/scsipi/atapi_wdc.c:1.131	Mon Oct 22 20:13:47 2018
+++ src/sys/dev/scsipi/atapi_wdc.c	Mon Nov 12 18:51:01 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: atapi_wdc.c,v 1.131 2018/10/22 20:13:47 jdolecek Exp $	*/
+/*	$NetBSD: atapi_wdc.c,v 1.132 2018/11/12 18:51:01 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.131 2018/10/22 20:13:47 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: atapi_wdc.c,v 1.132 2018/11/12 18:51:01 jdolecek Exp $");
 
 #ifndef ATADEBUG
 #define ATADEBUG
@@ -808,7 +808,6 @@ wdc_atapi_intr(struct ata_channel *chp, 
 		printf("%s:%d:%d: device timeout, c_bcount=%d, c_skip=%d\n",
 		    device_xname(atac->atac_dev), chp->ch_channel,
 		    xfer->c_drive, xfer->c_bcount, xfer->c_skip);
-		ata_channel_unlock(chp);
 #if NATA_DMA
 		if (xfer->c_flags & C_DMA) {
 			ata_dmaerr(drvp,
@@ -816,6 +815,7 @@ wdc_atapi_intr(struct ata_channel *chp, 
 		}
 #endif
 		sc_xfer->error = XS_TIMEOUT;
+		ata_channel_unlock(chp);
 		wdc_atapi_reset(chp, xfer);
 		return 1;
 	}
@@ -828,9 +828,9 @@ wdc_atapi_intr(struct ata_channel *chp, 
 	 * and reset device.
 	 */
 	if ((xfer->c_flags & C_TIMEOU) && (xfer->c_flags & C_DMA)) {
-		ata_channel_unlock(chp);
 		ata_dmaerr(drvp, (xfer->c_flags & C_POLL) ? AT_POLL : 0);
 		sc_xfer->error = XS_RESET;
+		ata_channel_unlock(chp);
 		wdc_atapi_reset(chp, xfer);
 		return (1);
 	}
@@ -908,12 +908,12 @@ again:
 		if ((sc_xfer->xs_control & XS_CTL_DATA_OUT) == 0 ||
 		    (xfer->c_flags & C_DMA) != 0) {
 			printf("wdc_atapi_intr: bad data phase DATAOUT\n");
-			ata_channel_unlock(chp);
 			if (xfer->c_flags & C_DMA) {
 				ata_dmaerr(drvp,
 				    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
 			}
 			sc_xfer->error = XS_TIMEOUT;
+			ata_channel_unlock(chp);
 			wdc_atapi_reset(chp, xfer);
 			return 1;
 		}
@@ -962,12 +962,12 @@ again:
 		if ((sc_xfer->xs_control & XS_CTL_DATA_IN) == 0 ||
 		    (xfer->c_flags & C_DMA) != 0) {
 			printf("wdc_atapi_intr: bad data phase DATAIN\n");
-			ata_channel_unlock(chp);
 			if (xfer->c_flags & C_DMA) {
 				ata_dmaerr(drvp,
 				    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
 			}
 			sc_xfer->error = XS_TIMEOUT;
+			ata_channel_unlock(chp);
 			wdc_atapi_reset(chp, xfer);
 			return 1;
 		}
@@ -1037,7 +1037,6 @@ again:
 			sc_xfer->error = XS_SHORTSENSE;
 			sc_xfer->sense.atapi_sense = ATACH_ERR(tfd);
 		} else {
-			ata_channel_unlock(chp);
 #if NATA_DMA
 			if (xfer->c_flags & C_DMA) {
 				ata_dmaerr(drvp,
@@ -1045,6 +1044,7 @@ again:
 			}
 #endif
 			sc_xfer->error = XS_RESET;
+			ata_channel_unlock(chp);
 			wdc_atapi_reset(chp, xfer);
 			return (1);
 		}
@@ -1123,12 +1123,12 @@ wdc_atapi_phase_complete(struct ata_xfer
 #if NATA_DMA || NATA_PIOBM
 		else if (wdc->dma_status &
 		    (WDC_DMAST_NOIRQ | WDC_DMAST_ERR)) {
-			ata_channel_unlock(chp);
 #if NATA_DMA
 			ata_dmaerr(drvp,
 			    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
 #endif
 			sc_xfer->error = XS_RESET;
+			ata_channel_unlock(chp);
 			wdc_atapi_reset(chp, xfer);
 			return;
 		}

Reply via email to