From owner-freebsd-bugs@FreeBSD.ORG Wed May 28 08:31:00 2003 Return-Path: Delivered-To: freebsd-bugs@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 30CA037B401; Wed, 28 May 2003 08:31:00 -0700 (PDT) Received: from comp.chem.msu.su (comp-ext.chem.msu.su [158.250.32.157]) by mx1.FreeBSD.org (Postfix) with ESMTP id CB7C043F75; Wed, 28 May 2003 08:30:57 -0700 (PDT) (envelope-from yar@comp.chem.msu.su) Received: from comp.chem.msu.su (localhost [127.0.0.1]) by comp.chem.msu.su (8.12.3p2/8.12.3) with ESMTP id h4SFUrSH035064; Wed, 28 May 2003 19:30:53 +0400 (MSD) (envelope-from yar@comp.chem.msu.su) Received: (from yar@localhost) by comp.chem.msu.su (8.12.3p2/8.12.3/Submit) id h4SFUedN035061; Wed, 28 May 2003 19:30:40 +0400 (MSD) (envelope-from yar) Date: Wed, 28 May 2003 19:30:40 +0400 From: Yar Tikhiy To: Bruce Evans Message-ID: <20030528153040.GA34472@comp.chem.msu.su> References: <200305161646.h4GGkdDS000677@stylish.chem.msu.su> <20030517165718.B15076@gamplex.bde.org> <20030517091047.GA82314@comp.chem.msu.su> <20030517235738.U16304@gamplex.bde.org> <20030519182357.GA99588@comp.chem.msu.su> <20030522220631.P43165@gamplex.bde.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20030522220631.P43165@gamplex.bde.org> User-Agent: Mutt/1.5.3i cc: freebsd-bugs@freebsd.org cc: FreeBSD-gnats-submit@freebsd.org cc: joerg@freebsd.org Subject: Re: kern/52338: fd(4) floppy disk driver & non-blocking I/O X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 May 2003 15:31:00 -0000 On Thu, May 22, 2003 at 10:19:20PM +1000, Bruce Evans wrote: > On Mon, 19 May 2003, Yar Tikhiy wrote: > > > On Sun, May 18, 2003 at 12:39:03AM +1000, Bruce Evans wrote: > > > > > > It should set bp->bio_resid (to bp->bio_bcount) (bio_resid and bio_bcount > > > are the same as b_resid and b_bcount here; strategy routines only have > > > access to a struct bio so they must use the former). > > > > What do you think about the following straightforward patch to fd.c? > > It catches all spots where BIO_ERROR is set, but bio_resid isn't. > > In fd.c, bio_resid is never set in advance, so the simplest approach > > should work. > > > > OTOH, I wonder if bio_resid could be set equal to bio_bcount at the > > beginning of fdstrategy() and changed only if needed. Does this have > > any obscure implications? > > I think that is the right place to set it. The residual count really > is the full count initially. Any obfuscations come later. The driver > might prefer to only set bio_resid to its final value at the end of > normal i/o instead of as every block is finished. Then the initial > setting would only used for abnormal i/o. This is essentially what > the fd driver does (except it just forgets to set bio_resid for abnormal > i/o). [...] > > --- fd.c.dist Fri Apr 11 15:39:24 2003 > > +++ fd.c Mon May 19 21:48:11 2003 > > @@ -1668,8 +1668,9 @@ fdstrategy(struct bio *bp) > > (u_long)major(bp->bio_dev), (u_long)minor(bp->bio_dev)); > > fdc = fd->fdc; > > if (fd->type == FDT_NONE || fd->ft == 0) { > > - bp->bio_error = ENXIO; > > + bp->bio_error = fd->type == FDT_NONE ? ENXIO : EAGAIN; > > bp->bio_flags |= BIO_ERROR; > > + bp->bio_resid = bp->bio_bcount; > > goto bad; > > } > > fdblk = 128 << (fd->ft->secsize); > > I think this should check FD_NONBLOCK like the next clause does, and only > return EAGAIN when FD_NONBLOCK is set. This is clearer even if it is > equivalent (can we get here with a type but no ft?). Right! Setting bio_resid at the beginning of fdstrategy() and checking for FD_NONBLOCK makes the change smaller and cleaner. I've attached the new version below. However, I'm unsure if I should push this into 5.1-RELEASE. As a matter of fact, this fixes an architectural bug that has (luckily) no impact on the system's functionality, unlike the related problem with panic on ioctl(). -- Yar --- fd.c.dist Fri Apr 11 15:39:24 2003 +++ fd.c Fri May 23 20:42:24 2003 @@ -1667,8 +1667,12 @@ fdstrategy(struct bio *bp) panic("fdstrategy: buf for nonexistent device (%#lx, %#lx)", (u_long)major(bp->bio_dev), (u_long)minor(bp->bio_dev)); fdc = fd->fdc; + bp->bio_resid = bp->bio_bcount; if (fd->type == FDT_NONE || fd->ft == 0) { - bp->bio_error = ENXIO; + if (fd->type != FDT_NONE && (fd->flags & FD_NONBLOCK)) + bp->bio_error = EAGAIN; + else + bp->bio_error = ENXIO; bp->bio_flags |= BIO_ERROR; goto bad; } @@ -1710,9 +1714,7 @@ fdstrategy(struct bio *bp) nblocks = fd->ft->size; if (blknum + bp->bio_bcount / fdblk > nblocks) { if (blknum >= nblocks) { - if (bp->bio_cmd == BIO_READ) - bp->bio_resid = bp->bio_bcount; - else { + if (bp->bio_cmd != BIO_READ) { bp->bio_error = ENOSPC; bp->bio_flags |= BIO_ERROR; }