Reading files on msdos-formated USB sticks under OpenBSD is really slow.
*One* of the reasons is that only one block is currently read-ahead if
possible.

Diff below converts msdosfs_read() to use bread_cluster() which at least
double the transfer rate when reading sequential blocks here.

When combined with the previous bread_cluster() diff, I get almost the
same read performances as with the raw device.

With a cheap USB device here, I have:

  Kernel:                       Max measure read speed:

  -current                               3.2M
  -current+this diff                     6.8M
  -current+this diff+bread_cluster      11.8M
  physio(9)                             12.4M

Index: msdosfs/msdosfs_vnops.c
===================================================================
RCS file: /cvs/src/sys/msdosfs/msdosfs_vnops.c,v
retrieving revision 1.104
diff -u -p -r1.104 msdosfs_vnops.c
--- msdosfs/msdosfs_vnops.c     23 Oct 2015 18:04:37 -0000      1.104
+++ msdosfs/msdosfs_vnops.c     3 Jan 2016 19:43:28 -0000
@@ -77,13 +77,13 @@
 
 static uint32_t fileidhash(uint64_t);
 
+int msdosfs_bmaparray(struct vnode *, daddr_t, daddr_t *, int *);
 int msdosfs_kqfilter(void *);
 int filt_msdosfsread(struct knote *, long);
 int filt_msdosfswrite(struct knote *, long);
 int filt_msdosfsvnode(struct knote *, long);
 void filt_msdosfsdetach(struct knote *);
 
-
 /*
  * Some general notes:
  *
@@ -511,18 +511,15 @@ int
 msdosfs_read(void *v)
 {
        struct vop_read_args *ap = v;
-       int error = 0;
-       uint32_t diff;
-       int blsize;
-       int isadir;
-       uint32_t n;
-       long on;
-       daddr_t lbn, rablock, rablkno;
-       struct buf *bp;
        struct vnode *vp = ap->a_vp;
        struct denode *dep = VTODE(vp);
        struct msdosfsmount *pmp = dep->de_pmp;
        struct uio *uio = ap->a_uio;
+       int size, isadir, error = 0;
+       daddr_t lbn;
+       uint32_t n, diff;
+       struct buf *bp;
+       long on;
 
        /*
         * If they didn't ask for any data, then we are done.
@@ -538,6 +535,7 @@ msdosfs_read(void *v)
                        return (0);
 
                lbn = de_cluster(pmp, uio->uio_offset);
+               size = pmp->pm_bpcluster;
                on = uio->uio_offset & pmp->pm_crbomask;
                n = min((uint32_t) (pmp->pm_bpcluster - on), uio->uio_resid);
 
@@ -550,37 +548,28 @@ msdosfs_read(void *v)
                if (diff < n)
                        n = diff;
 
-               /* convert cluster # to block # if a directory */
-               if (isadir) {
-                       error = pcbmap(dep, lbn, &lbn, 0, &blsize);
-                       if (error)
-                               return (error);
-               }
                /*
                 * If we are operating on a directory file then be sure to
                 * do i/o with the vnode for the filesystem instead of the
                 * vnode for the directory.
                 */
                if (isadir) {
-                       error = bread(pmp->pm_devvp, lbn, blsize, &bp);
+                       error = pcbmap(dep, lbn, &lbn, 0, &size);
+                       if (error)
+                               return (error);
+                       error = bread(pmp->pm_devvp, lbn, size, &bp);
                } else {
-                       rablock = lbn + 1;
-                       rablkno = de_cn2bn(pmp, rablock);
-                       if (dep->de_lastr + 1 == lbn &&
-                           de_cn2off(pmp, rablock) < dep->de_FileSize)
-                               error = breadn(vp, de_cn2bn(pmp, lbn),
-                                   pmp->pm_bpcluster, &rablkno,
-                                   &pmp->pm_bpcluster, 1, &bp);
+                       if (de_cn2off(pmp, lbn + 1) >= dep->de_FileSize)
+                               error = bread(vp, lbn, size, &bp);
                        else
-                               error = bread(vp, de_cn2bn(pmp, lbn),
-                                   pmp->pm_bpcluster, &bp);
-                       dep->de_lastr = lbn;
+                               error = bread_cluster(vp, lbn, size, &bp);
                }
-               n = min(n, pmp->pm_bpcluster - bp->b_resid);
                if (error) {
                        brelse(bp);
                        return (error);
                }
+
+               n = min(n, pmp->pm_bpcluster - bp->b_resid);
                error = uiomovei(bp->b_data + on, (int) n, uio);
                brelse(bp);
        } while (error == 0 && uio->uio_resid > 0 && n != 0);
@@ -701,8 +690,7 @@ msdosfs_write(void *v)
                         * for the fat table. (see msdosfs_strategy)
                         */
                        if (bp->b_blkno == bp->b_lblkno) {
-                               error = pcbmap(dep,
-                                              de_bn2cn(pmp, bp->b_lblkno),
+                               error = pcbmap(dep, bp->b_lblkno,
                                               &bp->b_blkno, 0, 0);
                                if (error)
                                        bp->b_blkno = -1;
@@ -1756,7 +1744,7 @@ msdosfs_islocked(void *v)
 
 /*
  * vp  - address of vnode file the file
- * bn  - which cluster we are interested in mapping to a filesystem block 
number.
+ * bn  - which cluster we are interested in mapping to a filesystem block 
number
  * vpp - returns the vnode for the block special file holding the filesystem
  *      containing the file of interest
  * bnp - address of where to return the filesystem relative block number
@@ -1766,19 +1754,48 @@ msdosfs_bmap(void *v)
 {
        struct vop_bmap_args *ap = v;
        struct denode *dep = VTODE(ap->a_vp);
-       struct msdosfsmount *pmp = dep->de_pmp;
 
        if (ap->a_vpp != NULL)
                *ap->a_vpp = dep->de_devvp;
        if (ap->a_bnp == NULL)
                return (0);
-       if (ap->a_runp) {
+
+       return (msdosfs_bmaparray(ap->a_vp, ap->a_bn, ap->a_bnp, ap->a_runp));
+}
+
+int
+msdosfs_bmaparray(struct vnode *vp, daddr_t bn, daddr_t *bnp, int *runp)
+{
+       struct denode *dep = VTODE(vp);
+       struct msdosfsmount *pmp = dep->de_pmp;
+       int error, maxrun = 0, run;
+       daddr_t daddr;
+
+       if (runp) {
                /*
-                * Sequential clusters should be counted here.
+                * XXX
+                * If MAXBSIZE is the largest transfer the disks can handle,
+                * we probably want maxrun to be 1 block less so that we
+                * don't create a block larger than the device can handle.
                 */
-               *ap->a_runp = 0;
+               *runp = 0;
+               maxrun = min(MAXBSIZE / pmp->pm_bpcluster - 1,
+                   pmp->pm_maxcluster - bn);
        }
-       return (pcbmap(dep, de_bn2cn(pmp, ap->a_bn), ap->a_bnp, 0, 0));
+
+       if ((error = pcbmap(dep, bn, bnp, 0, 0)) != 0)
+               return (error);
+
+       for (run = 1; run <= maxrun; run++) {
+               error = pcbmap(dep, bn + run, &daddr, 0, 0);
+               if (error != 0 || (daddr != *bnp + de_cn2bn(pmp, run)))
+                       break;
+       }
+
+       if (runp)
+               *runp = run - 1;
+
+       return (0);
 }
 
 int
@@ -1800,8 +1817,7 @@ msdosfs_strategy(void *v)
         * don't allow files with holes, so we shouldn't ever see this.
         */
        if (bp->b_blkno == bp->b_lblkno) {
-               error = pcbmap(dep, de_bn2cn(dep->de_pmp, bp->b_lblkno),
-                              &bp->b_blkno, 0, 0);
+               error = pcbmap(dep, bp->b_lblkno, &bp->b_blkno, 0, 0);
                if (error)
                        bp->b_blkno = -1;
                if (bp->b_blkno == -1)

Reply via email to