From owner-svn-src-projects@freebsd.org Tue Jun 25 17:24:45 2019 Return-Path: Delivered-To: svn-src-projects@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 423A415D2283 for ; Tue, 25 Jun 2019 17:24:45 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id E06626D5F5; Tue, 25 Jun 2019 17:24:44 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id B80FC1B88D; Tue, 25 Jun 2019 17:24:44 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x5PHOia8014884; Tue, 25 Jun 2019 17:24:44 GMT (envelope-from asomers@FreeBSD.org) Received: (from asomers@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x5PHOhN1014880; Tue, 25 Jun 2019 17:24:43 GMT (envelope-from asomers@FreeBSD.org) Message-Id: <201906251724.x5PHOhN1014880@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: asomers set sender to asomers@FreeBSD.org using -f From: Alan Somers Date: Tue, 25 Jun 2019 17:24:43 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r349378 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs X-SVN-Group: projects X-SVN-Commit-Author: asomers X-SVN-Commit-Paths: in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs X-SVN-Commit-Revision: 349378 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: E06626D5F5 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.97 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.97)[-0.966,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 Jun 2019 17:24:45 -0000 Author: asomers Date: Tue Jun 25 17:24:43 2019 New Revision: 349378 URL: https://svnweb.freebsd.org/changeset/base/349378 Log: fusefs: rewrite vop_getpages and vop_putpages Use the standard facilities for getpages and putpages instead of bespoke implementations that don't work well with the writeback cache. This has several corollaries: * Change the way we handle short reads _again_. vfs_bio_getpages doesn't provide any way to handle unexpected short reads. Plus, I found some more lock-order problems. So now when the short read is detected we'll just clear the vnode's attribute cache, forcing the file size to be requeried the next time it's needed. VOP_GETPAGES doesn't have any way to indicate a short read to the "caller", so we just bzero the rest of the page whenever a short read happens. * Change the way we decide when to set the FUSE_WRITE_CACHE bit. We now set it for clustered writes even when the writeback cache is not in use. Sponsored by: The FreeBSD Foundation Modified: projects/fuse2/sys/fs/fuse/fuse_io.c projects/fuse2/sys/fs/fuse/fuse_io.h projects/fuse2/sys/fs/fuse/fuse_vnops.c projects/fuse2/tests/sys/fs/fusefs/read.cc projects/fuse2/tests/sys/fs/fusefs/write.cc Modified: projects/fuse2/sys/fs/fuse/fuse_io.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_io.c Tue Jun 25 17:15:44 2019 (r349377) +++ projects/fuse2/sys/fs/fuse/fuse_io.c Tue Jun 25 17:24:43 2019 (r349378) @@ -100,6 +100,12 @@ __FBSDID("$FreeBSD$"); #include "fuse_ipc.h" #include "fuse_io.h" +/* + * Set in a struct buf to indicate that the write came from the buffer cache + * and the originating cred and pid are no longer known. + */ +#define B_FUSEFS_WRITE_CACHE B_FS_FLAG1 + SDT_PROVIDER_DECLARE(fusefs); /* * Fuse trace probe: @@ -164,10 +170,10 @@ fuse_io_clear_suid_on_write(struct vnode *vp, struct u SDT_PROBE_DEFINE5(fusefs, , io, io_dispatch, "struct vnode*", "struct uio*", "int", "struct ucred*", "struct fuse_filehandle*"); -SDT_PROBE_DEFINE5(fusefs, , io, io_dispatch_filehandles_closed, "struct vnode*", - "struct uio*", "int", "bool", "struct ucred*"); +SDT_PROBE_DEFINE4(fusefs, , io, io_dispatch_filehandles_closed, "struct vnode*", + "struct uio*", "int", "struct ucred*"); int -fuse_io_dispatch(struct vnode *vp, struct uio *uio, int ioflag, bool pages, +fuse_io_dispatch(struct vnode *vp, struct uio *uio, int ioflag, struct ucred *cred, pid_t pid) { struct fuse_filehandle *fufh; @@ -188,8 +194,8 @@ fuse_io_dispatch(struct vnode *vp, struct uio *uio, in closefufh = true; } else if (err) { - SDT_PROBE5(fusefs, , io, io_dispatch_filehandles_closed, - vp, uio, ioflag, pages, cred); + SDT_PROBE4(fusefs, , io, io_dispatch_filehandles_closed, + vp, uio, ioflag, cred); printf("FUSE: io dispatch: filehandles are closed\n"); return err; } @@ -236,15 +242,13 @@ fuse_io_dispatch(struct vnode *vp, struct uio *uio, in start = uio->uio_offset; end = start + uio->uio_resid; - /* - * Invalidate the write cache unless we're coming from - * VOP_PUTPAGES, in which case we're writing _from_ the - * write cache - */ - if (!pages ) - v_inval_buf_range(vp, start, end, iosize); + KASSERT((ioflag & (IO_VMIO | IO_DIRECT)) != + (IO_VMIO | IO_DIRECT), + ("IO_DIRECT used for a cache flush?")); + /* Invalidate the write cache when writing directly */ + v_inval_buf_range(vp, start, end, iosize); err = fuse_write_directbackend(vp, uio, cred, fufh, - filesize, ioflag, pages); + filesize, ioflag, false); } else { SDT_PROBE2(fusefs, , io, trace, 1, "buffered write of vnode"); @@ -352,8 +356,8 @@ fuse_read_biobackend(struct vnode *vp, struct uio *uio */ n = 0; - if (on < bcount - (intptr_t)bp->b_fsprivate1) - n = MIN((unsigned)(bcount - (intptr_t)bp->b_fsprivate1 - on), + if (on < bcount - bp->b_resid) + n = MIN((unsigned)(bcount - bp->b_resid - on), uio->uio_resid); if (n > 0) { SDT_PROBE2(fusefs, , io, read_bio_backend_feed, n, bp); @@ -362,10 +366,8 @@ fuse_read_biobackend(struct vnode *vp, struct uio *uio vfs_bio_brelse(bp, ioflag); SDT_PROBE4(fusefs, , io, read_bio_backend_end, err, uio->uio_resid, n, bp); - if ((intptr_t)bp->b_fsprivate1 > 0) { + if (bp->b_resid > 0) { /* Short read indicates EOF */ - (void)fuse_vnode_setsize(vp, uio->uio_offset); - bp->b_fsprivate1 = (void*)0; break; } } @@ -725,6 +727,21 @@ again: brelse(bp); break; } + if (bp->b_resid > 0) { + /* + * Short read indicates EOF. Update file size + * from the server and try again. + */ + SDT_PROBE2(fusefs, , io, trace, 1, + "Short read during a RMW"); + brelse(bp); + err = fuse_vnode_size(vp, &filesize, cred, + curthread); + if (err) + break; + else + goto again; + } } if (bp->b_wcred == NOCRED) bp->b_wcred = crhold(cred); @@ -805,8 +822,11 @@ again: vfs_bio_set_flags(bp, ioflag); + bp->b_flags |= B_FUSEFS_WRITE_CACHE; if (ioflag & IO_SYNC) { SDT_PROBE2(fusefs, , io, write_biobackend_issue, 2, bp); + if (!(ioflag & IO_VMIO)) + bp->b_flags &= ~B_FUSEFS_WRITE_CACHE; err = bwrite(bp); } else if (vm_page_count_severe() || buf_dirty_count_severe() || @@ -864,7 +884,6 @@ fuse_io_strategy(struct vnode *vp, struct buf *bp) fflag = bp->b_iocmd == BIO_READ ? FREAD : FWRITE; cred = bp->b_iocmd == BIO_READ ? bp->b_rcred : bp->b_wcred; error = fuse_filehandle_getrw(vp, fflag, &fufh, cred, pid); - bp->b_fsprivate1 = (void*)(intptr_t)0; if (bp->b_iocmd == BIO_READ && error == EBADF) { /* * This may be a read-modify-write operation on a cached file @@ -905,39 +924,40 @@ fuse_io_strategy(struct vnode *vp, struct buf *bp) uiop->uio_offset = ((off_t)bp->b_lblkno) * biosize; error = fuse_read_directbackend(vp, uiop, cred, fufh); - left = uiop->uio_resid; /* * Store the amount we failed to read in the buffer's private * field, so callers can truncate the file if necessary' */ - bp->b_fsprivate1 = (void*)(intptr_t)left; if (!error && uiop->uio_resid) { int nread = bp->b_bcount - uiop->uio_resid; + left = uiop->uio_resid; bzero((char *)bp->b_data + nread, left); - if (fuse_data_cache_mode != FUSE_CACHE_WB || - (fvdat->flag & FN_SIZECHANGE) == 0) { + if ((fvdat->flag & FN_SIZECHANGE) == 0) { /* * A short read with no error, when not using * direct io, and when no writes are cached, - * indicates EOF. Update the file size - * accordingly. We must still bzero the - * remaining buffer so uninitialized data - * doesn't get exposed by a future truncate - * that extends the file. + * indicates EOF caused by a server-side + * truncation. Clear the attr cache so we'll + * pick up the new file size and timestamps. + * + * We must still bzero the remaining buffer so + * uninitialized data doesn't get exposed by a + * future truncate that extends the file. * * To prevent lock order problems, we must - * truncate the file upstack + * truncate the file upstack, not here. */ SDT_PROBE2(fusefs, , io, trace, 1, "Short read of a clean file"); - uiop->uio_resid = 0; + fuse_vnode_clear_attr_cache(vp); } else { /* * If dirty writes _are_ cached beyond EOF, * that indicates a newly created hole that the - * server doesn't know about. + * server doesn't know about. Those don't pose + * any problem. * XXX: we don't currently track whether dirty * writes are cached beyond EOF, before EOF, or * both. @@ -976,8 +996,9 @@ fuse_io_strategy(struct vnode *vp, struct buf *bp) io.iov_base = (char *)bp->b_data + bp->b_dirtyoff; uiop->uio_rw = UIO_WRITE; + bool pages = bp->b_flags & B_FUSEFS_WRITE_CACHE; error = fuse_write_directbackend(vp, uiop, cred, fufh, - filesize, 0, false); + filesize, 0, pages); if (error == EINTR || error == ETIMEDOUT) { bp->b_flags &= ~(B_INVAL | B_NOCACHE); Modified: projects/fuse2/sys/fs/fuse/fuse_io.h ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_io.h Tue Jun 25 17:15:44 2019 (r349377) +++ projects/fuse2/sys/fs/fuse/fuse_io.h Tue Jun 25 17:24:43 2019 (r349378) @@ -60,7 +60,7 @@ #ifndef _FUSE_IO_H_ #define _FUSE_IO_H_ -int fuse_io_dispatch(struct vnode *vp, struct uio *uio, int ioflag, bool pages, +int fuse_io_dispatch(struct vnode *vp, struct uio *uio, int ioflag, struct ucred *cred, pid_t pid); int fuse_io_strategy(struct vnode *vp, struct buf *bp); int fuse_io_flushbuf(struct vnode *vp, int waitfor, struct thread *td); Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_vnops.c Tue Jun 25 17:15:44 2019 (r349377) +++ projects/fuse2/sys/fs/fuse/fuse_vnops.c Tue Jun 25 17:24:43 2019 (r349378) @@ -150,7 +150,6 @@ static vop_strategy_t fuse_vnop_strategy; static vop_symlink_t fuse_vnop_symlink; static vop_write_t fuse_vnop_write; static vop_getpages_t fuse_vnop_getpages; -static vop_putpages_t fuse_vnop_putpages; static vop_print_t fuse_vnop_print; static vop_vptofh_t fuse_vnop_vptofh; @@ -215,7 +214,6 @@ struct vop_vector fuse_vnops = { .vop_symlink = fuse_vnop_symlink, .vop_write = fuse_vnop_write, .vop_getpages = fuse_vnop_getpages, - .vop_putpages = fuse_vnop_putpages, .vop_print = fuse_vnop_print, .vop_vptofh = fuse_vnop_vptofh, }; @@ -1381,7 +1379,7 @@ fuse_vnop_read(struct vop_read_args *ap) ioflag |= IO_DIRECT; } - return fuse_io_dispatch(vp, uio, ioflag, false, cred, pid); + return fuse_io_dispatch(vp, uio, ioflag, cred, pid); } /* @@ -1960,229 +1958,60 @@ fuse_vnop_write(struct vop_write_args *ap) ioflag |= IO_DIRECT; } - return fuse_io_dispatch(vp, uio, ioflag, false, cred, pid); + return fuse_io_dispatch(vp, uio, ioflag, cred, pid); } -SDT_PROBE_DEFINE1(fusefs, , vnops, vnop_getpages_error, "int"); -/* - struct vnop_getpages_args { - struct vnode *a_vp; - vm_page_t *a_m; - int a_count; - int a_reqpage; - }; -*/ -static int -fuse_vnop_getpages(struct vop_getpages_args *ap) +static daddr_t +fuse_gbp_getblkno(struct vnode *vp, vm_ooffset_t off) { - int i, error, nextoff, size, toff, count, npages; - struct uio uio; - struct iovec iov; - vm_offset_t kva; - struct buf *bp; - struct vnode *vp; - struct thread *td; - struct ucred *cred; - vm_page_t *pages; - pid_t pid = curthread->td_proc->p_pid; + const int biosize = fuse_iosize(vp); - vp = ap->a_vp; - KASSERT(vp->v_object, ("objectless vp passed to getpages")); - td = curthread; /* XXX */ - cred = curthread->td_ucred; /* XXX */ - pages = ap->a_m; - npages = ap->a_count; + return (off / biosize); +} - if (!fsess_opt_mmap(vnode_mount(vp))) { - SDT_PROBE2(fusefs, , vnops, trace, 1, - "called on non-cacheable vnode??\n"); - return (VM_PAGER_ERROR); - } +static int +fuse_gbp_getblksz(struct vnode *vp, daddr_t lbn) +{ + off_t filesize; + int blksz, err; + const int biosize = fuse_iosize(vp); - /* - * If the last page is partially valid, just return it and allow - * the pager to zero-out the blanks. Partially valid pages can - * only occur at the file EOF. - * - * XXXGL: is that true for FUSE, which is a local filesystem, - * but still somewhat disconnected from the kernel? - */ - VM_OBJECT_WLOCK(vp->v_object); - if (pages[npages - 1]->valid != 0 && --npages == 0) - goto out; - VM_OBJECT_WUNLOCK(vp->v_object); + err = fuse_vnode_size(vp, &filesize, NULL, NULL); + KASSERT(err == 0, ("vfs_bio_getpages can't handle errors here")); + if (err) + return biosize; - /* - * We use only the kva address for the buffer, but this is extremely - * convenient and fast. - */ - bp = uma_zalloc(fuse_pbuf_zone, M_WAITOK); - - kva = (vm_offset_t)bp->b_data; - pmap_qenter(kva, pages, npages); - VM_CNT_INC(v_vnodein); - VM_CNT_ADD(v_vnodepgsin, npages); - - count = npages << PAGE_SHIFT; - iov.iov_base = (caddr_t)kva; - iov.iov_len = count; - uio.uio_iov = &iov; - uio.uio_iovcnt = 1; - uio.uio_offset = IDX_TO_OFF(pages[0]->pindex); - uio.uio_resid = count; - uio.uio_segflg = UIO_SYSSPACE; - uio.uio_rw = UIO_READ; - uio.uio_td = td; - - error = fuse_io_dispatch(vp, &uio, IO_DIRECT, true, cred, pid); - pmap_qremove(kva, npages); - - uma_zfree(fuse_pbuf_zone, bp); - - if (error && (uio.uio_resid == count)) { - SDT_PROBE1(fusefs, , vnops, vnop_getpages_error, error); - return VM_PAGER_ERROR; + if ((off_t)lbn * biosize >= filesize) { + blksz = 0; + } else if ((off_t)(lbn + 1) * biosize > filesize) { + blksz = filesize - (off_t)lbn *biosize; + } else { + blksz = biosize; } - /* - * Calculate the number of bytes read and validate only that number - * of bytes. Note that due to pending writes, size may be 0. This - * does not mean that the remaining data is invalid! - */ - - size = count - uio.uio_resid; - VM_OBJECT_WLOCK(vp->v_object); - fuse_vm_page_lock_queues(); - for (i = 0, toff = 0; i < npages; i++, toff = nextoff) { - vm_page_t m; - - nextoff = toff + PAGE_SIZE; - m = pages[i]; - - if (nextoff <= size) { - /* - * Read operation filled an entire page - */ - m->valid = VM_PAGE_BITS_ALL; - KASSERT(m->dirty == 0, - ("fuse_getpages: page %p is dirty", m)); - } else if (size > toff) { - /* - * Read operation filled a partial page. - */ - m->valid = 0; - vm_page_set_valid_range(m, 0, size - toff); - KASSERT(m->dirty == 0, - ("fuse_getpages: page %p is dirty", m)); - } else { - /* - * Read operation was short. If no error occurred - * we may have hit a zero-fill section. We simply - * leave valid set to 0. - */ - ; - } - } - fuse_vm_page_unlock_queues(); -out: - VM_OBJECT_WUNLOCK(vp->v_object); - if (ap->a_rbehind) - *ap->a_rbehind = 0; - if (ap->a_rahead) - *ap->a_rahead = 0; - return (VM_PAGER_OK); + return (blksz); } /* - struct vnop_putpages_args { + struct vnop_getpages_args { struct vnode *a_vp; vm_page_t *a_m; int a_count; - int a_sync; - int *a_rtvals; - vm_ooffset_t a_offset; + int a_reqpage; }; */ static int -fuse_vnop_putpages(struct vop_putpages_args *ap) +fuse_vnop_getpages(struct vop_getpages_args *ap) { - struct uio uio; - struct iovec iov; - vm_offset_t kva; - struct buf *bp; - int i, error, npages, count; - off_t offset; - int *rtvals; - struct vnode *vp; - struct thread *td; - struct ucred *cred; - vm_page_t *pages; - vm_ooffset_t fsize; - pid_t pid = curthread->td_proc->p_pid; + struct vnode *vp = ap->a_vp; - vp = ap->a_vp; - KASSERT(vp->v_object, ("objectless vp passed to putpages")); - fsize = vp->v_object->un_pager.vnp.vnp_size; - td = curthread; /* XXX */ - cred = curthread->td_ucred; /* XXX */ - pages = ap->a_m; - count = ap->a_count; - rtvals = ap->a_rtvals; - npages = btoc(count); - offset = IDX_TO_OFF(pages[0]->pindex); - if (!fsess_opt_mmap(vnode_mount(vp))) { SDT_PROBE2(fusefs, , vnops, trace, 1, "called on non-cacheable vnode??\n"); + return (VM_PAGER_ERROR); } - for (i = 0; i < npages; i++) - rtvals[i] = VM_PAGER_AGAIN; - /* - * When putting pages, do not extend file past EOF. - */ - - if (offset + count > fsize) { - count = fsize - offset; - if (count < 0) - count = 0; - } - /* - * We use only the kva address for the buffer, but this is extremely - * convenient and fast. - */ - bp = uma_zalloc(fuse_pbuf_zone, M_WAITOK); - - kva = (vm_offset_t)bp->b_data; - pmap_qenter(kva, pages, npages); - VM_CNT_INC(v_vnodeout); - VM_CNT_ADD(v_vnodepgsout, count); - - iov.iov_base = (caddr_t)kva; - iov.iov_len = count; - uio.uio_iov = &iov; - uio.uio_iovcnt = 1; - uio.uio_offset = offset; - uio.uio_resid = count; - uio.uio_segflg = UIO_SYSSPACE; - uio.uio_rw = UIO_WRITE; - uio.uio_td = td; - - error = fuse_io_dispatch(vp, &uio, IO_DIRECT, true, cred, pid); - - pmap_qremove(kva, npages); - uma_zfree(fuse_pbuf_zone, bp); - - if (!error) { - int nwritten = round_page(count - uio.uio_resid) / PAGE_SIZE; - - for (i = 0; i < nwritten; i++) { - rtvals[i] = VM_PAGER_OK; - VM_OBJECT_WLOCK(pages[i]->object); - vm_page_undirty(pages[i]); - VM_OBJECT_WUNLOCK(pages[i]->object); - } - } - return rtvals[0]; + return (vfs_bio_getpages(vp, ap->a_m, ap->a_count, ap->a_rbehind, + ap->a_rahead, fuse_gbp_getblkno, fuse_gbp_getblksz)); } static const char extattr_namespace_separator = '.'; Modified: projects/fuse2/tests/sys/fs/fusefs/read.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/read.cc Tue Jun 25 17:15:44 2019 (r349377) +++ projects/fuse2/tests/sys/fs/fusefs/read.cc Tue Jun 25 17:24:43 2019 (r349378) @@ -413,7 +413,8 @@ TEST_F(Read, eio) /* * If the server returns a short read when direct io is not in use, that - * indicates EOF and we should update the file size. + * indicates EOF, because of a server-side truncation. We should invalidate + * all cached attributes. We may update the file size, */ TEST_F(ReadCacheable, eof) { @@ -425,18 +426,21 @@ TEST_F(ReadCacheable, eof) uint64_t offset = 100; ssize_t bufsize = strlen(CONTENTS); ssize_t partbufsize = 3 * bufsize / 4; + ssize_t r; char buf[bufsize]; struct stat sb; expect_lookup(RELPATH, ino, offset + bufsize); expect_open(ino, 0, 1); expect_read(ino, 0, offset + bufsize, offset + partbufsize, CONTENTS); + expect_getattr(ino, offset + partbufsize); fd = open(FULLPATH, O_RDONLY); ASSERT_LE(0, fd) << strerror(errno); - ASSERT_EQ(partbufsize, pread(fd, buf, bufsize, offset)) - << strerror(errno); + r = pread(fd, buf, bufsize, offset); + ASSERT_LE(0, r) << strerror(errno); + EXPECT_EQ(partbufsize, r) << strerror(errno); ASSERT_EQ(0, fstat(fd, &sb)); EXPECT_EQ((off_t)(offset + partbufsize), sb.st_size); /* Deliberately leak fd. close(2) will be tested in release.cc */ @@ -459,6 +463,7 @@ TEST_F(ReadCacheable, eof_of_whole_buffer) expect_open(ino, 0, 1); expect_read(ino, 2 * m_maxbcachebuf, bufsize, bufsize, CONTENTS); expect_read(ino, m_maxbcachebuf, m_maxbcachebuf, 0, CONTENTS); + expect_getattr(ino, m_maxbcachebuf); fd = open(FULLPATH, O_RDONLY); ASSERT_LE(0, fd) << strerror(errno); @@ -581,6 +586,57 @@ TEST_F(ReadCacheable, mmap) ASSERT_NE(MAP_FAILED, p) << strerror(errno); ASSERT_EQ(0, memcmp(p, CONTENTS, bufsize)); + + ASSERT_EQ(0, munmap(p, len)) << strerror(errno); + /* Deliberately leak fd. close(2) will be tested in release.cc */ +} + +/* + * A read via mmap comes up short, indicating that the file was truncated + * server-side. + */ +TEST_F(ReadCacheable, mmap_eof) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const char *CONTENTS = "abcdefgh"; + uint64_t ino = 42; + int fd; + ssize_t len; + size_t bufsize = strlen(CONTENTS); + struct stat sb; + void *p; + + len = getpagesize(); + + expect_lookup(RELPATH, ino, 100000); + expect_open(ino, 0, 1); + /* mmap may legitimately try to read more data than is available */ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in.header.opcode == FUSE_READ && + in.header.nodeid == ino && + in.body.read.fh == Read::FH && + in.body.read.offset == 0 && + in.body.read.size >= bufsize); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + out.header.len = sizeof(struct fuse_out_header) + bufsize; + memmove(out.body.bytes, CONTENTS, bufsize); + }))); + expect_getattr(ino, bufsize); + + fd = open(FULLPATH, O_RDONLY); + ASSERT_LE(0, fd) << strerror(errno); + + p = mmap(NULL, len, PROT_READ, MAP_SHARED, fd, 0); + ASSERT_NE(MAP_FAILED, p) << strerror(errno); + + /* The file size should be automatically truncated */ + ASSERT_EQ(0, memcmp(p, CONTENTS, bufsize)); + ASSERT_EQ(0, fstat(fd, &sb)) << strerror(errno); + EXPECT_EQ((off_t)bufsize, sb.st_size); ASSERT_EQ(0, munmap(p, len)) << strerror(errno); /* Deliberately leak fd. close(2) will be tested in release.cc */ Modified: projects/fuse2/tests/sys/fs/fusefs/write.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/write.cc Tue Jun 25 17:15:44 2019 (r349377) +++ projects/fuse2/tests/sys/fs/fusefs/write.cc Tue Jun 25 17:24:43 2019 (r349378) @@ -528,6 +528,39 @@ TEST_F(Write, rlimit_fsize) /* Deliberately leak fd. close(2) will be tested in release.cc */ } +/* + * A short read indicates EOF. Test that nothing bad happens if we get EOF + * during the R of a RMW operation. + */ +TEST_F(WriteCacheable, eof_during_rmw) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const char *CONTENTS = "abcdefgh"; + const char *INITIAL = "XXXXXXXXXX"; + uint64_t ino = 42; + uint64_t offset = 1; + ssize_t bufsize = strlen(CONTENTS); + off_t orig_fsize = 10; + off_t truncated_fsize = 5; + off_t final_fsize = bufsize; + int fd; + + FuseTest::expect_lookup(RELPATH, ino, S_IFREG | 0644, orig_fsize, 1); + expect_open(ino, 0, 1); + expect_read(ino, 0, orig_fsize, truncated_fsize, INITIAL, O_RDWR); + expect_getattr(ino, truncated_fsize); + expect_read(ino, 0, final_fsize, final_fsize, INITIAL, O_RDWR); + maybe_expect_write(ino, offset, bufsize, CONTENTS); + + fd = open(FULLPATH, O_RDWR); + EXPECT_LE(0, fd) << strerror(errno); + + ASSERT_EQ(bufsize, pwrite(fd, CONTENTS, bufsize, offset)) + << strerror(errno); + /* Deliberately leak fd. close(2) will be tested in release.cc */ +} + /* * If the kernel cannot be sure which uid, gid, or pid was responsible for a * write, then it must set the FUSE_WRITE_CACHE bit