From owner-freebsd-current@FreeBSD.ORG Mon Aug 2 17:08:21 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id A2D5E16A4CE for ; Mon, 2 Aug 2004 17:08:21 +0000 (GMT) Received: from will.iki.fi (will.iki.fi [217.169.64.20]) by mx1.FreeBSD.org (Postfix) with ESMTP id 18A8443D68 for ; Mon, 2 Aug 2004 17:08:21 +0000 (GMT) (envelope-from will+freebsd-current@will.iki.fi) Received: from [10.129.39.131] (b212-54-23-216.elisa-laajakaista.fi [212.54.23.216]) by will.iki.fi (Postfix) with ESMTP id C13DC1F3; Mon, 2 Aug 2004 20:08:17 +0300 (EEST) Message-ID: <410E74F7.1070000@will.iki.fi> Date: Mon, 02 Aug 2004 20:08:07 +0300 From: Ville-Pertti Keinonen User-Agent: Mozilla Thunderbird 0.7.1 (X11/20040708) X-Accept-Language: en-us, en MIME-Version: 1.0 To: freebsd-current@freebsd.org References: <410E688D.7020709@will.iki.fi> In-Reply-To: <410E688D.7020709@will.iki.fi> Content-Type: multipart/mixed; boundary="------------030208070903070201030302" cc: =?ISO-8859-1?Q?S=F8ren_Schmidt?= Subject: Re: ATA driver races with interrupts X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Aug 2004 17:08:21 -0000 This is a multi-part message in MIME format. --------------030208070903070201030302 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit My previous patch makes things more reliable, but it's not good enough... Attached is an updated version of the patch that clears ATA_EXPECT_INTR earlier (in the interrupt handler), which seems to stabilize things better. I'm definitely not convinced of its correctness, since I'm not sure which part of the code is racing against interrupts despite the previous version of the patch, but I can no longer cause any failures. Repeating the problem should be trivial with an unpatched -current with PREEMPTION enabled and hardware similar to mine - ASUS K8V Deluxe, two SATA disks on the VIA 6420: ... atapci1: port 0xd000-0xd0ff,0xd400-0xd40f,0xd800-0xd803,0xe000-0xe007,0xe400-0xe403,0xe800-0xe807 irq 20 at device 15.0 on pci0 ata5: at 0xe800 on atapci1 ata6: at 0xe000 on atapci1 ... ad10: 152627MB [310101/16/63] at ata5-master SATA150 ad12: 152627MB [310101/16/63] at ata6-master SATA150 Partitions from ad10 and ad12 were mounted on /sata1 and /sata2, respectively, both containing tens of gigabytes of files. By issuing $ find /sata1 -type f -exec md5 '{}' \; & find /sata2 -type f -exec md5 '{}' \; & the system would lock up or detach one of the disks in less than 5 seconds. With the attached version of my patch, it completed successfully. --------------030208070903070201030302 Content-Type: text/plain; name="atafix3.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="atafix3.diff" Index: ata-all.h =================================================================== RCS file: /data/freebsd/src/sys/dev/ata/ata-all.h,v retrieving revision 1.79 diff -u -r1.79 ata-all.h --- ata-all.h 30 Apr 2004 16:21:34 -0000 1.79 +++ ata-all.h 2 Aug 2004 15:54:50 -0000 @@ -339,6 +339,7 @@ #define ATA_48BIT_ACTIVE 0x10 #define ATA_IMMEDIATE_MODE 0x20 #define ATA_HWGONE 0x40 +#define ATA_EXPECT_INTR 0x80 struct ata_device device[2]; /* devices on this channel */ #define MASTER 0x00 Index: ata-lowlevel.c =================================================================== RCS file: /data/freebsd/src/sys/dev/ata/ata-lowlevel.c,v retrieving revision 1.40 diff -u -r1.40 ata-lowlevel.c --- ata-lowlevel.c 24 Jul 2004 19:03:28 -0000 1.40 +++ ata-lowlevel.c 2 Aug 2004 16:34:00 -0000 @@ -81,6 +81,7 @@ } /* record the request as running */ + ch->flags &= ~ATA_EXPECT_INTR; ch->running = request; ATA_DEBUG_RQ(request, "transaction"); @@ -140,6 +141,7 @@ } /* return and wait for interrupt */ + ch->flags |= ATA_EXPECT_INTR; return ATA_OP_CONTINUES; /* ATA DMA data transfer commands */ @@ -169,6 +171,7 @@ } /* return and wait for interrupt */ + ch->flags |= ATA_EXPECT_INTR; return ATA_OP_CONTINUES; /* ATAPI PIO commands */ @@ -225,6 +228,7 @@ ATA_PROTO_ATAPI_12 ? 6 : 8); /* return and wait for interrupt */ + ch->flags |= ATA_EXPECT_INTR; return ATA_OP_CONTINUES; case ATA_R_ATAPI|ATA_R_DMA: @@ -290,6 +294,7 @@ } /* return and wait for interrupt */ + ch->flags |= ATA_EXPECT_INTR; return ATA_OP_CONTINUES; } @@ -308,7 +313,7 @@ int length; /* ignore this interrupt if there is no running request */ - if (!request) + if (!request || !(ch->flags & ATA_EXPECT_INTR)) return; ATA_DEBUG_RQ(request, "interrupt"); @@ -524,6 +529,8 @@ break; } + ch->flags &= ~ATA_EXPECT_INTR; + /* if we timed out the unlocking of the ATA channel is done later */ if (!(request->flags & ATA_R_TIMEOUT)) { ch->running = NULL; --------------030208070903070201030302--