From owner-svn-src-projects@freebsd.org Sat Mar 30 00:57:09 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 E593A155909E for ; Sat, 30 Mar 2019 00:57:08 +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 8C5EF8AA46; Sat, 30 Mar 2019 00:57:08 +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 5FCB26F73; Sat, 30 Mar 2019 00:57:08 +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 x2U0v89Y088362; Sat, 30 Mar 2019 00:57:08 GMT (envelope-from asomers@FreeBSD.org) Received: (from asomers@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x2U0v7qp088358; Sat, 30 Mar 2019 00:57:07 GMT (envelope-from asomers@FreeBSD.org) Message-Id: <201903300057.x2U0v7qp088358@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: asomers set sender to asomers@FreeBSD.org using -f From: Alan Somers Date: Sat, 30 Mar 2019 00:57:07 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r345722 - 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: 345722 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 8C5EF8AA46 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.95 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; NEURAL_HAM_SHORT(-0.96)[-0.955,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US]; NEURAL_HAM_LONG(-1.00)[-1.000,0] 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: Sat, 30 Mar 2019 00:57:09 -0000 Author: asomers Date: Sat Mar 30 00:57:07 2019 New Revision: 345722 URL: https://svnweb.freebsd.org/changeset/base/345722 Log: fusefs: don't force direct io for files opened O_WRONLY Previously fusefs would treat any file opened O_WRONLY as though the FOPEN_DIRECT_IO flag were set, in an attempt to avoid issuing reads as part of a RMW write operation on a cached part of the file. However, the FUSE protocol explicitly allows reads of write-only files for precisely that reason. Sponsored by: The FreeBSD Foundation Modified: projects/fuse2/sys/fs/fuse/fuse_file.c projects/fuse2/sys/fs/fuse/fuse_io.c projects/fuse2/sys/fs/fuse/fuse_vnops.c projects/fuse2/tests/sys/fs/fusefs/write.cc Modified: projects/fuse2/sys/fs/fuse/fuse_file.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_file.c Sat Mar 30 00:54:01 2019 (r345721) +++ projects/fuse2/sys/fs/fuse/fuse_file.c Sat Mar 30 00:57:07 2019 (r345722) @@ -144,16 +144,7 @@ fuse_filehandle_open(struct vnode *vp, fufh_type_t fuf fuse_filehandle_init(vp, fufh_type, fufhp, foo->fh); - /* - * For WRONLY opens, force DIRECT_IO. This is necessary - * since writing a partial block through the buffer cache - * will result in a read of the block and that read won't - * be allowed by the WRONLY open. - */ - if (fufh_type == FUFH_WRONLY) - fuse_vnode_open(vp, foo->open_flags | FOPEN_DIRECT_IO, td); - else - fuse_vnode_open(vp, foo->open_flags, td); + fuse_vnode_open(vp, foo->open_flags, td); out: fdisp_destroy(&fdi); Modified: projects/fuse2/sys/fs/fuse/fuse_io.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_io.c Sat Mar 30 00:54:01 2019 (r345721) +++ projects/fuse2/sys/fs/fuse/fuse_io.c Sat Mar 30 00:57:07 2019 (r345722) @@ -648,6 +648,15 @@ fuse_io_strategy(struct vnode *vp, struct buf *bp) error = fuse_filehandle_getrw(vp, (bp->b_iocmd == BIO_READ) ? FUFH_RDONLY : FUFH_WRONLY, &fufh); + if (bp->b_iocmd == BIO_READ && error == EBADF) { + /* + * This may be a read-modify-write operation on a cached file + * opened O_WRONLY. The FUSE protocol allows this. + * + * TODO: eliminate this hacky check once the FUFH table is gone + */ + error = fuse_filehandle_get(vp, FUFH_WRONLY, &fufh); + } if (error) { printf("FUSE: strategy: filehandles are closed\n"); bp->b_ioflags |= BIO_ERROR; Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_vnops.c Sat Mar 30 00:54:01 2019 (r345721) +++ projects/fuse2/sys/fs/fuse/fuse_vnops.c Sat Mar 30 00:57:07 2019 (r345722) @@ -1208,7 +1208,6 @@ fuse_vnop_open(struct vop_open_args *ap) int mode = ap->a_mode; struct thread *td = ap->a_td; struct ucred *cred = ap->a_cred; - int32_t fuse_open_flags = 0; fufh_type_t fufh_type; struct fuse_vnode_data *fvdat; @@ -1228,16 +1227,8 @@ fuse_vnop_open(struct vop_open_args *ap) fufh_type = fuse_filehandle_xlate_from_fflags(mode); } - /* - * For WRONLY opens, force DIRECT_IO. This is necessary since writing - * a partial block through the buffer cache will result in a read of - * the block and that read won't be allowed by the WRONLY open. - */ - if (fufh_type == FUFH_WRONLY || (fvdat->flag & FN_DIRECTIO) != 0) - fuse_open_flags = FOPEN_DIRECT_IO; - if (fuse_filehandle_validrw(vp, fufh_type) != FUFH_INVALID) { - fuse_vnode_open(vp, fuse_open_flags, td); + fuse_vnode_open(vp, 0, td); return 0; } Modified: projects/fuse2/tests/sys/fs/fusefs/write.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/write.cc Sat Mar 30 00:54:01 2019 (r345721) +++ projects/fuse2/tests/sys/fs/fusefs/write.cc Sat Mar 30 00:57:07 2019 (r345722) @@ -391,7 +391,7 @@ TEST_F(Write, DISABLED_mmap) free(zeros); } -TEST_F(Write, pwrite) +TEST_F(WriteThrough, pwrite) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; @@ -519,6 +519,36 @@ TEST_F(WriteBack, close) ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno); close(fd); +} + +/* + * In writeback mode, writes to an O_WRONLY file could trigger reads from the + * server. The FUSE protocol explicitly allows that. + */ +TEST_F(WriteBack, 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; + off_t fsize = 10; + int fd; + ssize_t bufsize = strlen(CONTENTS); + + expect_lookup(RELPATH, ino, 0); + expect_open(ino, 0, 1); + expect_getattr(ino, fsize); + expect_read(ino, 0, fsize, fsize, INITIAL); + expect_write(ino, offset, bufsize, bufsize, 0, CONTENTS); + + fd = open(FULLPATH, O_WRONLY); + 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 */ } /*