From f4aaab5fd4219e21322d2fe25905a711d91e97da Mon Sep 17 00:00:00 2001 From: sf Date: Mon, 29 May 2017 13:48:12 +0000 Subject: [PATCH] Implement VFS read clustering for MSDOSFS This is the reverted commit by mpi@ from msdosfs_vnops.c 1.105 plus some additional tweaks to fix some cluster/block number confusion that lead to regressions when seeking past the end of a file. The original commit message was: The logic used in msdosfs_bmap() to loop calling pcbmap() comes from FreeBSD and is not really efficient but it is good enough since it is only called when generating I/O. With this diff I get a 100% improvement when reading big files from a crappy USB stick. With this and bread_cluster(9) modified to not re-fetch B_CACHED buffers, reading large contiguous files with chunk sizes of MAXPHYS is almost as fast as physio(9) on the same device. For a 'real world' example, when copying music files from a USB stick I see a speed jump from 15MB/s on -current to 24Mb/s with this diff. While here rename some 'lbn' variables into 'cn' to better reflect what we're dealing with. Tested by Mathieu, with support from deraadt@ ok mpi@ --- sys/msdosfs/denode.h | 3 +- sys/msdosfs/msdosfs_vnops.c | 108 ++++++++++++++++++++++-------------- 2 files changed, 67 insertions(+), 44 deletions(-) diff --git a/sys/msdosfs/denode.h b/sys/msdosfs/denode.h index cdca90500ab..efa8192a06d 100644 --- a/sys/msdosfs/denode.h +++ b/sys/msdosfs/denode.h @@ -1,4 +1,4 @@ -/* $OpenBSD: denode.h,v 1.30 2016/08/30 19:47:23 sf Exp $ */ +/* $OpenBSD: denode.h,v 1.31 2017/05/29 13:48:12 sf Exp $ */ /* $NetBSD: denode.h,v 1.24 1997/10/17 11:23:39 ws Exp $ */ /*- @@ -142,7 +142,6 @@ struct denode { struct vnode *de_devvp; /* vnode of blk dev we live on */ uint32_t de_flag; /* flag bits */ dev_t de_dev; /* device where direntry lives */ - daddr_t de_lastr; uint32_t de_dirclust; /* cluster of the directory file containing this entry */ uint32_t de_diroffset; /* offset of this entry in the directory cluster */ uint32_t de_fndoffset; /* offset of found dir entry */ diff --git a/sys/msdosfs/msdosfs_vnops.c b/sys/msdosfs/msdosfs_vnops.c index 39c60044df5..61b45af6185 100644 --- a/sys/msdosfs/msdosfs_vnops.c +++ b/sys/msdosfs/msdosfs_vnops.c @@ -1,4 +1,4 @@ -/* $OpenBSD: msdosfs_vnops.c,v 1.113 2016/08/30 19:47:23 sf Exp $ */ +/* $OpenBSD: msdosfs_vnops.c,v 1.114 2017/05/29 13:48:12 sf Exp $ */ /* $NetBSD: msdosfs_vnops.c,v 1.63 1997/10/17 11:24:19 ws Exp $ */ /*- @@ -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: * @@ -509,18 +509,14 @@ 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 isadir, error = 0; + uint32_t n, diff, size, on; + struct buf *bp; + daddr_t cn, bn; /* * If they didn't ask for any data, then we are done. @@ -535,7 +531,8 @@ msdosfs_read(void *v) if (uio->uio_offset >= dep->de_FileSize) return (0); - lbn = de_cluster(pmp, uio->uio_offset); + cn = de_cluster(pmp, uio->uio_offset); + size = pmp->pm_bpcluster; on = uio->uio_offset & pmp->pm_crbomask; n = ulmin(pmp->pm_bpcluster - on, uio->uio_resid); @@ -548,31 +545,22 @@ 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, cn, &cn, 0, &size); + if (error) + return (error); + error = bread(pmp->pm_devvp, cn, 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); + bn = de_cn2bn(pmp, cn); + if (de_cn2off(pmp, cn + 1) >= dep->de_FileSize) + error = bread(vp, bn, size, &bp); else - error = bread(vp, de_cn2bn(pmp, lbn), - pmp->pm_bpcluster, &bp); - dep->de_lastr = lbn; + error = bread_cluster(vp, bn, size, &bp); } n = min(n, pmp->pm_bpcluster - bp->b_resid); if (error) { @@ -601,7 +589,7 @@ msdosfs_write(void *v) uint32_t osize; int error = 0; uint32_t count, lastcn; - daddr_t bn; + daddr_t cn, bn; struct buf *bp; int ioflag = ap->a_ioflag; struct uio *uio = ap->a_uio; @@ -678,20 +666,23 @@ msdosfs_write(void *v) lastcn = de_clcount(pmp, osize) - 1; do { - if (de_cluster(pmp, uio->uio_offset) > lastcn) { + croffset = uio->uio_offset & pmp->pm_crbomask; + cn = de_cluster(pmp, uio->uio_offset); + + if (cn > lastcn) { error = ENOSPC; break; } - bn = de_blk(pmp, uio->uio_offset); - if ((uio->uio_offset & pmp->pm_crbomask) == 0 - && (de_blk(pmp, uio->uio_offset + uio->uio_resid) > de_blk(pmp, uio->uio_offset) - || uio->uio_offset + uio->uio_resid >= dep->de_FileSize)) { + if (croffset == 0 && + (de_cluster(pmp, uio->uio_offset + uio->uio_resid) > cn || + (uio->uio_offset + uio->uio_resid) >= dep->de_FileSize)) { /* * If either the whole cluster gets written, * or we write the cluster from its start beyond EOF, * then no need to read data from disk. */ + bn = de_cn2bn(pmp, cn); bp = getblk(thisvp, bn, pmp->pm_bpcluster, 0, 0); clrbuf(bp); /* @@ -713,8 +704,10 @@ msdosfs_write(void *v) } } else { /* - * The block we need to write into exists, so read it in. + * The block we need to write into exists, so + * read it in. */ + bn = de_cn2bn(pmp, cn); error = bread(thisvp, bn, pmp->pm_bpcluster, &bp); if (error) { brelse(bp); @@ -722,7 +715,6 @@ msdosfs_write(void *v) } } - croffset = uio->uio_offset & pmp->pm_crbomask; n = ulmin(uio->uio_resid, pmp->pm_bpcluster - croffset); if (uio->uio_offset + n > dep->de_FileSize) { dep->de_FileSize = uio->uio_offset + n; @@ -1755,7 +1747,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 @@ -1765,19 +1757,51 @@ 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 cn, daddr_t *bnp, int *runp) +{ + struct denode *dep = VTODE(vp); + struct msdosfsmount *pmp = dep->de_pmp; + struct mount *mp; + int error, maxrun = 0, run; + daddr_t daddr; + + mp = vp->v_mount; + + 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 / mp->mnt_stat.f_iosize - 1, + pmp->pm_maxcluster - cn); } - return (pcbmap(dep, de_bn2cn(pmp, ap->a_bn), ap->a_bnp, 0, 0)); + + if ((error = pcbmap(dep, cn, bnp, 0, 0)) != 0) + return (error); + + for (run = 1; run <= maxrun; run++) { + error = pcbmap(dep, cn + run, &daddr, 0, 0); + if (error != 0 || (daddr != *bnp + de_cn2bn(pmp, run))) + break; + } + + if (runp) + *runp = run - 1; + + return (0); } int -- 2.20.1