kern/52338: fd(4) floppy disk driver & non-blocking I/O

[Yar Tikhiy]

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;
 			}