On 29/05/17(Mon) 14:15, Stefan Fritsch wrote:
> Last year, mpi@ implemented VFS read clustering for MSDOSFS in
>
> sys/msdosfs/denode.h 1.28
> sys/msdosfs/msdosfs_vnops.c 1.105
>
> This caused regressions when doing seeks past the end of the file and had
> to be reverted.
>
> I have now written a test that catches the bug (committed in
> regress/sys/fileops) and got a fix that does not break the test:
>
> There was some confusion as to what block sizes are used in file
> operations. Openbsd seems to be more like netbsd than like freebsd.
>
> See msdosfs_vnops.c 1.62 in freebsd:
>
> Author: msmith <msm...@freebsd.org>
> Date: Sun Mar 1 21:26:09 1998 +0000
>
> Fix mmap() on msdosfs. In the words of the submitter:
>
> |In the process of evaluating the getpages/putpages issues I discovered
> |that mmap on MSDOSFS does not work. This is because I blindly merged
> |NetBSD changes in msdosfs_bmap and msdosfs_strategy. Apparently, their
> |blocksize is always DEV_BSIZE (even in files), while in FreeBSD
> |blocksize in files is v_mount->mnt_stat.f_iosize (i.e. clustersize in
> |MSDOSFS case). The patch is below.
>
> Submitted by: Dmitrij Tejblum <d...@tejblum.dnttm.rssi.ru>
>
> The diff is meant to be on top of mpi's diff (or on top of a revert of the
> revert 1.113). I can send the complete diff if that's preferred.
A complete diff would be easier to review.
>
> diff --git a/sys/msdosfs/msdosfs_vnops.c b/sys/msdosfs/msdosfs_vnops.c
> index 45a479fb646..46ff2736ca4 100644
> --- sys/msdosfs/msdosfs_vnops.c
> +++ sys/msdosfs/msdosfs_vnops.c
> @@ -516,7 +516,7 @@ msdosfs_read(void *v)
> int isadir, error = 0;
> uint32_t n, diff, size, on;
> struct buf *bp;
> - daddr_t cn;
> + daddr_t cn, bn;
>
> /*
> * If they didn't ask for any data, then we are done.
> @@ -556,10 +556,11 @@ msdosfs_read(void *v)
> return (error);
> error = bread(pmp->pm_devvp, cn, size, &bp);
> } else {
> + bn = de_cn2bn(pmp, cn);
> if (de_cn2off(pmp, cn + 1) >= dep->de_FileSize)
> - error = bread(vp, cn, size, &bp);
> + error = bread(vp, bn, size, &bp);
> else
> - error = bread_cluster(vp, cn, size, &bp);
> + error = bread_cluster(vp, bn, size, &bp);
> }
> n = min(n, pmp->pm_bpcluster - bp->b_resid);
> if (error) {
> @@ -588,7 +589,7 @@ msdosfs_write(void *v)
> uint32_t osize;
> int error = 0;
> uint32_t count, lastcn;
> - daddr_t cn;
> + daddr_t cn, bn;
> struct buf *bp;
> int ioflag = ap->a_ioflag;
> struct uio *uio = ap->a_uio;
> @@ -681,18 +682,19 @@ msdosfs_write(void *v)
> * or we write the cluster from its start beyond EOF,
> * then no need to read data from disk.
> */
> - bp = getblk(thisvp, cn, pmp->pm_bpcluster, 0, 0);
> + bn = de_cn2bn(pmp, cn);
> + bp = getblk(thisvp, bn, pmp->pm_bpcluster, 0, 0);
> clrbuf(bp);
> /*
> * Do the bmap now, since pcbmap needs buffers
> * for the fat table. (see msdosfs_strategy)
> */
> if (bp->b_blkno == bp->b_lblkno) {
> - error = pcbmap(dep, bp->b_lblkno, &cn, 0, 0);
> + error = pcbmap(dep,
> + de_bn2cn(pmp, bp->b_lblkno),
> + &bp->b_blkno, 0, 0);
> if (error)
> bp->b_blkno = -1;
> - else
> - bp->b_blkno = cn;
> }
> if (bp->b_blkno == -1) {
> brelse(bp);
> @@ -705,7 +707,8 @@ msdosfs_write(void *v)
> * The block we need to write into exists, so
> * read it in.
> */
> - error = bread(thisvp, cn, pmp->pm_bpcluster, &bp);
> + bn = de_cn2bn(pmp, cn);
> + error = bread(thisvp, bn, pmp->pm_bpcluster, &bp);
> if (error) {
> brelse(bp);
> break;
> @@ -1820,7 +1823,8 @@ 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, bp->b_lblkno, &bp->b_blkno, 0, 0);
> + error = pcbmap(dep, de_bn2cn(dep->de_pmp, bp->b_lblkno),
> + &bp->b_blkno, 0, 0);
> if (error)
> bp->b_blkno = -1;
> if (bp->b_blkno == -1)
>
