Re: kern/38643: [dM] st tape drive loses data

[der Mouse <mouse%Rodents-Montreal.ORG@localhost>]

The following reply was made to PR kern/38643; it has been noted by GNATS.

From: der Mouse <mouse%Rodents-Montreal.ORG@localhost>
To: Manuel Bouyer <bouyer%antioche.eu.org@localhost>, Havard Eidnes 
<he%NetBSD.org@localhost>,
        gnats-bugs%NetBSD.org@localhost, kern-bug-people%NetBSD.org@localhost,
        gnats-admin%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost, 
yamt%NetBSD.org@localhost
Cc: 
Subject: Re: kern/38643: [dM] st tape drive loses data
Date: Mon, 8 Sep 2008 16:05:42 -0400 (EDT)

 Today, I tried adding printfs in stread and ststrategy.  These indicate
 that, even when stread provides a buf, physio dispatches two requests
 to the strategy routine:
 
 stread: offset 0 resid 1048577
 ststrategy: bcount 65536 blkno 0
 ststrategy: bcount 65536 blkno 128
 stread: offset 10240 resid 10240
 ststrategy: bcount 10240 blkno 20
 
 So it appears there is something wrong with physio.  After staring at
 it for a while, I came up with this
 
 --- /dev/fd/4  Tue Dec  9 16:18:08 2003
 +++ /dev/fd/5  Tue Dec  9 16:18:08 2003
 @@ -336,6 +336,9 @@
                        if (error) {
                                goto done_locked;
                        }
 +                      if ((mbp->b_flags & B_ERROR) != 0) {
 +                              goto done_locked;
 +                      }
                        simple_unlock(&mbp->b_interlock);
                        if (obp != NULL) {
                                /*
 
 which appears to fix this.  (Otherwise, the main physio line blocks in
 physio_wait before the first request comes off the workqueue and sets
 B_ERROR in mbp, and the B_ERROR isn't noticed until after dispatching
 the second request.)  Combine this with making st.c use its own buffer
 in stread and stwrite
 
 --- /dev/fd/4  Tue Dec  9 16:18:08 2003
 +++ /dev/fd/5  Tue Dec  9 16:18:08 2003
 @@ -1379,8 +1379,9 @@
  stread(dev_t dev, struct uio *uio, int iomode)
  {
        struct st_softc *st = st_cd.cd_devs[STUNIT(dev)];
 +      static struct buf buf;
  
 -      return (physio(ststrategy, NULL, dev, B_READ,
 +      return (physio(ststrategy, &buf, dev, B_READ,
            st->sc_periph->periph_channel->chan_adapter->adapt_minphys, uio));
  }
  
 @@ -1388,8 +1389,9 @@
  stwrite(dev_t dev, struct uio *uio, int iomode)
  {
        struct st_softc *st = st_cd.cd_devs[STUNIT(dev)];
 +      static struct buf buf;
  
 -      return (physio(ststrategy, NULL, dev, B_WRITE,
 +      return (physio(ststrategy, &buf, dev, B_WRITE,
            st->sc_periph->periph_channel->chan_adapter->adapt_minphys, uio));
  }
  
 
 and, in initial tests, st is fine.  I'll have to run with this for a
 while to see if there are any subtle problems remaining.
 
 /~\ The ASCII                          der Mouse
 \ / Ribbon Campaign
  X  Against HTML               mouse%rodents-montreal.org@localhost
 / \ Email!          7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B