[patch] MFI should set bio_resid on command failure

[Mark Johnston]

On Tue, Jul 10, 2012 at 12:59:07PM -0700, Scott Long wrote:
> 
> 
> 
> 
> ----- Original Message -----
> > From: Sean Bruno <seanbru at yahoo-inc.com>
> > To: Andrew Boyer <aboyer at averesystems.com>
> > Cc: "freebsd-scsi at freebsd.org" <freebsd-scsi at freebsd.org>; scottl at freebsd.org; "ambrisko at FreeBSD.ORG" <ambrisko at freebsd.org>
> > Sent: Tuesday, July 10, 2012 10:43 AM
> > Subject: Re: [patch] MFI should set bio_resid on command failure
> > 
> > On Tue, 2012-07-10 at 09:37 -0700, Andrew Boyer wrote:
> >>  On Jul 3, 2012, at 2:41 PM, Sean Bruno wrote:
> >> 
> >>  > On Tue, 2012-07-03 at 06:47 -0700, Andrew Boyer wrote:
> >>  >> When an MFI command fails, the driver needs to set 
> > bio->bio_resid so that the upper levels notice.  Otherwise we see commands 
> > silently failing leading to data corruption.  This mirrors dadone().
> >>  >> 
> >>  >> -Andrew
> >>  >> 
> >>  >> Index: sys/dev/mfi/mfi_disk.c
> >>  >> 
> > ===================================================================
> >>  >> --- sys/dev/mfi/mfi_disk.c    (revision 238071)
> >>  >> +++ sys/dev/mfi/mfi_disk.c    (working copy)
> >>  >> @@ -298,6 +298,7 @@
> >>  >>     hdr = bio->bio_driver1;
> >>  >> 
> >>  >>     if (bio->bio_flags & BIO_ERROR) {
> >>  >> +        bio->bio_resid = bio->bio_bcount;
> >>  >>         if (bio->bio_error == 0)
> >>  >>             bio->bio_error = EIO;
> >>  >>         disk_err(bio, "hard error", -1, 1);
> >>  >> 
> >>  >> --------------------------------------------------
> >>  >> Andrew Boyer    aboyer at averesystems.com
> >>  >> 
> >>  > 
> >>  > This looks right to me.  It mirrors the behavior in 
> > mfi_disk_strategy()
> >>  > as well.
> >>  > 
> >>  > Sean
> >>  > 
> >> 
> >>  Anyone interested in committing?  (And MFC'ing to stable/8…)
> >> 
> >>  -Andrew
> >> 
> >>  --------------------------------------------------
> >>  Andrew Boyer    aboyer at averesystems.com
> >> 
> >> 
> >> 
> > 
> > Let me hit dougA and scottl for validation.  Their wisdom here can
> > validate my naivete.
> > 
> 
> 
> Yeah, b_resid needs to be set, and a lot of drivers get this wrong.
> 
> Scott

It seems that the analogous line is also missing from aac(4). Is the
patch below correct? Or am I missing something?

Thanks,
-Mark

diff --git a/sys/dev/aac/aac_disk.c b/sys/dev/aac/aac_disk.c
index e4042f7..f2f6636 100644
--- a/sys/dev/aac/aac_disk.c
+++ b/sys/dev/aac/aac_disk.c
@@ -334,8 +334,10 @@ aac_biodone(struct bio *bp)
 {
 	fwprintf(NULL, HBA_FLAGS_DBG_FUNCTION_ENTRY_B, "");
 
-	if (bp->bio_flags & BIO_ERROR)
+	if (bp->bio_flags & BIO_ERROR) {
+		bp->bio_resid = bp->bio_bcount;
 		disk_err(bp, "hard error", -1, 1);
+	}
 
 	biodone(bp);
 }