From owner-svn-src-projects@freebsd.org Thu Apr 11 05:11:04 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 25B001579943 for ; Thu, 11 Apr 2019 05:11:04 +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 BA2E07620C; Thu, 11 Apr 2019 05:11:03 +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 ABC525752; Thu, 11 Apr 2019 05:11:03 +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 x3B5B3C0026369; Thu, 11 Apr 2019 05:11:03 GMT (envelope-from asomers@FreeBSD.org) Received: (from asomers@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x3B5B3iN026366; Thu, 11 Apr 2019 05:11:03 GMT (envelope-from asomers@FreeBSD.org) Message-Id: <201904110511.x3B5B3iN026366@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: asomers set sender to asomers@FreeBSD.org using -f From: Alan Somers Date: Thu, 11 Apr 2019 05:11:03 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r346117 - 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: 346117 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: BA2E07620C X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.98 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-0.998,0]; NEURAL_HAM_SHORT(-0.98)[-0.982,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: Thu, 11 Apr 2019 05:11:04 -0000 Author: asomers Date: Thu Apr 11 05:11:02 2019 New Revision: 346117 URL: https://svnweb.freebsd.org/changeset/base/346117 Log: fusefs: eliminate a superfluous FUSE_GETATTR from VOP_LOOKUP fuse_vnop_lookup was using a FUSE_GETATTR operation when looking up "." and "..", even though the only information it needed was the file type and file size. "." and ".." are obviously always going to be directories; there's no need to double check. Sponsored by: The FreeBSD Foundation Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c projects/fuse2/tests/sys/fs/fusefs/lookup.cc projects/fuse2/tests/sys/fs/fusefs/xattr.cc Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_vnops.c Thu Apr 11 05:08:49 2019 (r346116) +++ projects/fuse2/sys/fs/fuse/fuse_vnops.c Thu Apr 11 05:11:02 2019 (r346117) @@ -734,10 +734,10 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) struct vnode *vp = NULL; struct fuse_dispatcher fdi; - enum fuse_opcode op; + bool did_lookup = false; struct fuse_entry_out *feo = NULL; - struct fuse_attr_out *fao = NULL; - struct fuse_attr *fattr = NULL; + enum vtype vtyp; /* vnode type of target */ + off_t filesize; /* filesize of target */ uint64_t nid; @@ -756,17 +756,16 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) if (flags & ISDOTDOT) { nid = VTOFUD(dvp)->parent_nid; - if (nid == 0) { + if (nid == 0) return ENOENT; - } - fdisp_init(&fdi, 0); - op = FUSE_GETATTR; - goto calldaemon; + /* .. is obviously a directory */ + vtyp = VDIR; + filesize = 0; } else if (cnp->cn_namelen == 1 && *(cnp->cn_nameptr) == '.') { nid = VTOI(dvp); - fdisp_init(&fdi, 0); - op = FUSE_GETATTR; - goto calldaemon; + /* . is obviously a directory */ + vtyp = VDIR; + filesize = 0; } else { struct timespec now, timeout; @@ -806,44 +805,42 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) default: return err; } - } - nid = VTOI(dvp); - fdisp_init(&fdi, cnp->cn_namelen + 1); - op = FUSE_LOOKUP; -calldaemon: - fdisp_make(&fdi, op, mp, nid, td, cred); + nid = VTOI(dvp); + fdisp_init(&fdi, cnp->cn_namelen + 1); + fdisp_make(&fdi, FUSE_LOOKUP, mp, nid, td, cred); - if (op == FUSE_LOOKUP) { memcpy(fdi.indata, cnp->cn_nameptr, cnp->cn_namelen); ((char *)fdi.indata)[cnp->cn_namelen] = '\0'; - } - lookup_err = fdisp_wait_answ(&fdi); + lookup_err = fdisp_wait_answ(&fdi); + did_lookup = true; - if ((op == FUSE_LOOKUP) && !lookup_err) { - /* lookup call succeeded */ - nid = ((struct fuse_entry_out *)fdi.answ)->nodeid; - if (nid == 0) { - /* zero nodeid means ENOENT and cache it */ - struct timespec timeout; + if (!lookup_err) { + /* lookup call succeeded */ + nid = ((struct fuse_entry_out *)fdi.answ)->nodeid; + feo = (struct fuse_entry_out *)fdi.answ; + if (nid == 0) { + /* zero nodeid means ENOENT and cache it */ + struct timespec timeout; - fdi.answ_stat = ENOENT; - lookup_err = ENOENT; - if (cnp->cn_flags & MAKEENTRY) { - feo = (struct fuse_entry_out *)fdi.answ; - fuse_validity_2_timespec(feo, &timeout); - cache_enter_time(dvp, *vpp, cnp, &timeout, - NULL); + fdi.answ_stat = ENOENT; + lookup_err = ENOENT; + if (cnp->cn_flags & MAKEENTRY) { + fuse_validity_2_timespec(feo, &timeout); + cache_enter_time(dvp, *vpp, cnp, + &timeout, NULL); + } + } else if (nid == FUSE_ROOT_ID) { + lookup_err = EINVAL; } - } else if (nid == FUSE_ROOT_ID) { - lookup_err = EINVAL; + vtyp = IFTOVT(feo->attr.mode); + filesize = feo->attr.size; } + if (lookup_err && (!fdi.answ_stat || lookup_err != ENOENT)) { + fdisp_destroy(&fdi); + return lookup_err; + } } - if (lookup_err && - (!fdi.answ_stat || lookup_err != ENOENT || op != FUSE_LOOKUP)) { - fdisp_destroy(&fdi); - return lookup_err; - } /* lookup_err, if non-zero, must be ENOENT at this point */ if (lookup_err) { @@ -871,13 +868,6 @@ calldaemon: } else { /* Entry was found */ - if (op == FUSE_GETATTR) { - fattr = &((struct fuse_attr_out *)fdi.answ)->attr; - } else { - feo = (struct fuse_entry_out *)fdi.answ; - fattr = &(feo->attr); - } - if ((nameiop == DELETE || nameiop == RENAME) && islastcn) { err = fuse_internal_access(dvp, VWRITE, td, cred); if (err != 0) @@ -912,7 +902,7 @@ calldaemon: *vpp = dvp; } else { err = fuse_vnode_get(dvp->v_mount, feo, nid, - dvp, &vp, cnp, IFTOVT(fattr->mode)); + dvp, &vp, cnp, vtyp); if (err) goto out; *vpp = vp; @@ -941,7 +931,7 @@ calldaemon: goto out; } err = fuse_vnode_get(vnode_mount(dvp), feo, nid, dvp, - &vp, cnp, IFTOVT(fattr->mode)); + &vp, cnp, vtyp); if (err) { goto out; } @@ -980,7 +970,7 @@ calldaemon: } VOP_UNLOCK(dvp, 0); err = fuse_vnode_get(vnode_mount(dvp), feo, nid, NULL, - &vp, cnp, IFTOVT(fattr->mode)); + &vp, cnp, vtyp); vfs_unbusy(mp); vn_lock(dvp, ltype | LK_RETRY); if ((dvp->v_iflag & VI_DOOMED) != 0) { @@ -998,7 +988,7 @@ calldaemon: struct fuse_vnode_data *fvdat; err = fuse_vnode_get(vnode_mount(dvp), feo, nid, dvp, - &vp, cnp, IFTOVT(fattr->mode)); + &vp, cnp, vtyp); if (err) { goto out; } @@ -1016,7 +1006,7 @@ calldaemon: */ fvdat = VTOFUD(vp); if (vnode_isreg(vp) && - fattr->size != fvdat->filesize) { + filesize != fvdat->filesize) { /* * The FN_SIZECHANGE flag reflects a dirty * append. If userspace lets us know our cache @@ -1031,21 +1021,15 @@ calldaemon: "%s!\n", __func__, vnode_mount(vp)->mnt_stat.f_mntonname); - (void)fuse_vnode_setsize(vp, cred, fattr->size); + (void)fuse_vnode_setsize(vp, cred, filesize); fvdat->flag &= ~FN_SIZECHANGE; } *vpp = vp; } - if (op == FUSE_GETATTR) { - fao = (struct fuse_attr_out*)fdi.answ; - fuse_internal_cache_attrs(*vpp, - &fao->attr, fao->attr_valid, - fao->attr_valid_nsec, NULL); - } else { - fuse_internal_cache_attrs(*vpp, - &feo->attr, feo->attr_valid, - feo->attr_valid_nsec, NULL); + if (feo != NULL) { + fuse_internal_cache_attrs(*vpp, &feo->attr, + feo->attr_valid, feo->attr_valid_nsec, NULL); } } out: @@ -1054,16 +1038,17 @@ out: /* No lookup error; need to clean up. */ if (err) { /* Found inode; exit with no vnode. */ - if (op == FUSE_LOOKUP) { - fuse_internal_forget_send(vnode_mount(dvp), td, cred, - nid, 1); + if (feo != NULL) { + fuse_internal_forget_send(vnode_mount(dvp), td, + cred, nid, 1); } - fdisp_destroy(&fdi); + if (did_lookup) + fdisp_destroy(&fdi); return err; - } else { } } - fdisp_destroy(&fdi); + if (did_lookup) + fdisp_destroy(&fdi); return err; } Modified: projects/fuse2/tests/sys/fs/fusefs/lookup.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/lookup.cc Thu Apr 11 05:08:49 2019 (r346116) +++ projects/fuse2/tests/sys/fs/fusefs/lookup.cc Thu Apr 11 05:11:02 2019 (r346117) @@ -134,6 +134,49 @@ TEST_F(Lookup, attr_cache_timeout) ASSERT_EQ(0, stat(FULLPATH, &sb)) << strerror(errno); } +TEST_F(Lookup, dot) +{ + const char FULLPATH[] = "mountpoint/some_dir/."; + const char RELDIRPATH[] = "some_dir"; + uint64_t ino = 42; + + EXPECT_LOOKUP(1, RELDIRPATH) + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) { + SET_OUT_HEADER_LEN(out, entry); + out->body.entry.attr.mode = S_IFDIR | 0755; + out->body.entry.nodeid = ino; + out->body.entry.attr_valid = UINT64_MAX; + out->body.entry.entry_valid = UINT64_MAX; + }))); + + /* + * access(2) is one of the few syscalls that will not (always) follow + * up a successful VOP_LOOKUP with another VOP. + */ + ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno); +} + +TEST_F(Lookup, dotdot) +{ + const char FULLPATH[] = "mountpoint/some_dir/.."; + const char RELDIRPATH[] = "some_dir"; + + EXPECT_LOOKUP(1, RELDIRPATH) + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) { + SET_OUT_HEADER_LEN(out, entry); + out->body.entry.attr.mode = S_IFDIR | 0755; + out->body.entry.nodeid = 14; + out->body.entry.attr_valid = UINT64_MAX; + out->body.entry.entry_valid = UINT64_MAX; + }))); + + /* + * access(2) is one of the few syscalls that will not (always) follow + * up a successful VOP_LOOKUP with another VOP. + */ + ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno); +} + TEST_F(Lookup, enoent) { const char FULLPATH[] = "mountpoint/does_not_exist"; Modified: projects/fuse2/tests/sys/fs/fusefs/xattr.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/xattr.cc Thu Apr 11 05:08:49 2019 (r346116) +++ projects/fuse2/tests/sys/fs/fusefs/xattr.cc Thu Apr 11 05:11:02 2019 (r346117) @@ -636,5 +636,3 @@ TEST_F(RofsXattr, setextattr_erofs) ASSERT_EQ(-1, r); EXPECT_EQ(EROFS, errno); } - -// TODO: EROFS tests