From owner-freebsd-usb@FreeBSD.ORG Thu Dec 27 19:14:43 2007 Return-Path: Delivered-To: freebsd-usb@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6651016A41A for ; Thu, 27 Dec 2007 19:14:43 +0000 (UTC) (envelope-from henrik@gulbra.net) Received: from av7-2-sn3.vrr.skanova.net (av7-2-sn3.vrr.skanova.net [81.228.9.182]) by mx1.freebsd.org (Postfix) with ESMTP id 02AD313C46E for ; Thu, 27 Dec 2007 19:14:42 +0000 (UTC) (envelope-from henrik@gulbra.net) Received: by av7-2-sn3.vrr.skanova.net (Postfix, from userid 502) id 3CC20387EB; Thu, 27 Dec 2007 19:45:20 +0100 (CET) Received: from smtp3-2-sn3.vrr.skanova.net (smtp3-2-sn3.vrr.skanova.net [81.228.9.102]) by av7-2-sn3.vrr.skanova.net (Postfix) with ESMTP id 2436338139; Thu, 27 Dec 2007 19:45:20 +0100 (CET) Received: from [192.168.0.2] (81-235-156-87-no29.tbcn.telia.com [81.235.156.87]) by smtp3-2-sn3.vrr.skanova.net (Postfix) with ESMTP id A691137E4F; Thu, 27 Dec 2007 19:47:57 +0100 (CET) From: Henrik Gulbrandsen To: "M. Warner Losh" In-Reply-To: <20071226.174523.1326317191.imp@bsdimp.com> References: <1198689316.1119.382.camel@Particle> <20071226.114812.1649771240.imp@bsdimp.com> <1198713403.1119.414.camel@Particle> <20071226.174523.1326317191.imp@bsdimp.com> Content-Type: text/plain Date: Thu, 27 Dec 2007 19:46:25 +0100 Message-Id: <1198781185.1119.478.camel@Particle> Mime-Version: 1.0 X-Mailer: Evolution 2.12.1 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: freebsd-usb@FreeBSD.org Subject: READ_CAPACITY_OFFBY1 [was: Re: usb/umass, devfs: this sucks] X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Dec 2007 19:14:43 -0000 I'm dropping the stable list and continuing discussion on the USB list. On Wed, 2007-12-26 at 11:48 -0700, M. Warner Losh wrote: > The patch in [usb/78984] is wrong. There are devices that are an odd > number of sectors. This is one of the places where the obvious patch > isn't necessarily right. On Wed, 2007-12-26 at 17:45 -0700, M. Warner Losh wrote: > There's already a quirk for this problem that has been applied to the > few devices that it affects. That would be the new READ_CAPACITY_OFFBY1, which you checked in about five months after my patch submission :-) > Maybe we need to add one more to the list? There was also talk of > forcing a read to the last sector if the sector count was odd, but it > never got past talk. Well, there are at least three ways of handling this, but none of them seems particularly good to me. It all boils down to a trade-off: a) Using the quirk flag. This is the obvious solution, now when it has been added to the umass.c file. The problem with quirks in general is highlighted by your check-in comment: "...there are a host of other devices with this issue, including iPods and some popular cameras". Each of these will be separately debugged and added to the list, and since the link between cause and effect can be less than obvious in this case, that can take hours of work per device. b) Reading from the last sector. This is an interesting solution, but worries me, because that sector will technically be out-of-range in the devices that we're interested in. If they can't get the sector count right, how will they handle a read from an invalid sector? I'm not a bit surprised if there are devices out there that would block indefinitely, convert the read to a write in the first sector and corrupt the file system, or generally burn the barn... c) Using heuristics. This is what I did when I assumed that there should typically be an even number of sectors. While that is obviously a bit too optimistic, perhaps there are other sufficiently good algorithms for detecting the incorrect cases with a negligible number of false positives. It may be due to a lack of imagination, but I find it hard to think of any reason why somebody would put exactly 256001 512-byte sectors on a umass device, unless that's an off-by-one error in itself. For my own personal use, solution (a) is good enough, but I'd prefer a more general solution, so we can use scarce developer time in a more useful way than for identifying this quirk over and over again. /Henrik