From owner-freebsd-bugs@FreeBSD.ORG Thu May 22 05:19:29 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 0417A37B401; Thu, 22 May 2003 05:19:29 -0700 (PDT) Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by mx1.FreeBSD.org (Postfix) with ESMTP id 24DDF43F75; Thu, 22 May 2003 05:19:27 -0700 (PDT) (envelope-from bde@zeta.org.au) Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246]) by mailman.zeta.org.au (8.9.3p2/8.8.7) with ESMTP id WAA25740; Thu, 22 May 2003 22:19:22 +1000 Date: Thu, 22 May 2003 22:19:20 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Yar Tikhiy In-Reply-To: <20030519182357.GA99588@comp.chem.msu.su> Message-ID: <20030522220631.P43165@gamplex.bde.org> References: <200305161646.h4GGkdDS000677@stylish.chem.msu.su> <20030517091047.GA82314@comp.chem.msu.su> <20030519182357.GA99588@comp.chem.msu.su> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII 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: Thu, 22 May 2003 12:19:29 -0000 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). > And my other thought is: What if physio() sets bio_resid equal to > bio_bcount before calling DEV_STRATEGY()? Currently, physio() > leaves bio_resid unset. An obvious drawback of this approach is > that it would encourage poor coding in drivers, though. Rev.1.37 of kern/subr_disk.c sets bio_resid to bio_bcount in the generic strategy routine. This is a better place than physio(). The fd driver didn't benefit from this because it never used the disk mini-layer. Now geom is used instead of the disk mini-layer. geom now uses essentially the same method as the fd driver for normal i/o -- it keeps track of the amount of i/o completed and subtracts this from bio_bcount to get bio_resid when i/o completes, and it doesn't set bio_resid initially. The fd driver doesn't benefit from this because it doesn't use geom either. > --- 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?). Bruce