Date: Thu, 8 Nov 2012 00:32:50 +0000 (UTC) From: Attilio Rao <attilio@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r242727 - head/sys/fs/fuse Message-ID: <201211080032.qA80Wo6Z039813@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: attilio Date: Thu Nov 8 00:32:49 2012 New Revision: 242727 URL: http://svnweb.freebsd.org/changeset/base/242727 Log: - Current caching mode is completely broken because it simply relies on timing of the operations and not real lookup, bringing too many false positives. Remove the whole mechanism. If it needs to be implemented, next time it should really be done in the proper way. - Fix VOP_GETATTR() in order to cope with userland bugs that would change the type of file and not panic. Instead it gets the entry as if it is not existing. Reported and tested by: flo MFC after: 2 months X-MFC: 241519, 242536,242616 Modified: head/sys/fs/fuse/fuse_file.c head/sys/fs/fuse/fuse_internal.c head/sys/fs/fuse/fuse_internal.h head/sys/fs/fuse/fuse_io.c head/sys/fs/fuse/fuse_node.c head/sys/fs/fuse/fuse_node.h head/sys/fs/fuse/fuse_vnops.c Modified: head/sys/fs/fuse/fuse_file.c ============================================================================== --- head/sys/fs/fuse/fuse_file.c Thu Nov 8 00:32:36 2012 (r242726) +++ head/sys/fs/fuse/fuse_file.c Thu Nov 8 00:32:49 2012 (r242727) @@ -192,7 +192,6 @@ out: atomic_subtract_acq_int(&fuse_fh_count, 1); fufh->fh_id = (uint64_t)-1; fufh->fh_type = FUFH_INVALID; - fuse_invalidate_attr(vp); return err; } Modified: head/sys/fs/fuse/fuse_internal.c ============================================================================== --- head/sys/fs/fuse/fuse_internal.c Thu Nov 8 00:32:36 2012 (r242726) +++ head/sys/fs/fuse/fuse_internal.c Thu Nov 8 00:32:49 2012 (r242727) @@ -371,26 +371,6 @@ fuse_internal_readdir_processdata(struct /* remove */ -#ifdef XXXIP -static int -fuse_internal_remove_callback(struct vnode *vp, void *cargs) -{ - struct vattr *vap; - uint64_t target_nlink; - - vap = VTOVA(vp); - - target_nlink = *(uint64_t *)cargs; - - /* somewhat lame "heuristics", but you got better ideas? */ - if ((vap->va_nlink == target_nlink) && vnode_isreg(vp)) { - fuse_invalidate_attr(vp); - } - return 0; -} - -#endif - #define INVALIDATE_CACHED_VATTRS_UPON_UNLINK 1 int fuse_internal_remove(struct vnode *dvp, @@ -426,27 +406,6 @@ fuse_internal_remove(struct vnode *dvp, err = fdisp_wait_answ(&fdi); fdisp_destroy(&fdi); - - fuse_invalidate_attr(dvp); - fuse_invalidate_attr(vp); - -#ifdef XXXIP - /* - * XXX: INVALIDATE_CACHED_VATTRS_UPON_UNLINK - * - * Consider the case where vap->va_nlink > 1 for the entity being - * removed. In our world, other in-memory vnodes that share a link - * count each with this one may not know right way that this one just - * got deleted. We should let them know, say, through a vnode_iterate() - * here and a callback that does fuse_invalidate_attr(vp) on each - * relevant vnode. - */ - if (need_invalidate && !err) { - vnode_iterate(vnode_mount(vp), 0, fuse_internal_remove_callback, - (void *)&target_nlink); - } -#endif - return err; } @@ -477,11 +436,6 @@ fuse_internal_rename(struct vnode *fdvp, err = fdisp_wait_answ(&fdi); fdisp_destroy(&fdi); - - fuse_invalidate_attr(fdvp); - if (tdvp != fdvp) { - fuse_invalidate_attr(tdvp); - } return err; } @@ -556,7 +510,6 @@ fuse_internal_newentry(struct vnode *dvp bufsize, &fdi); err = fuse_internal_newentry_core(dvp, vpp, cnp, vtype, &fdi); fdisp_destroy(&fdi); - fuse_invalidate_attr(dvp); return err; } Modified: head/sys/fs/fuse/fuse_internal.h ============================================================================== --- head/sys/fs/fuse/fuse_internal.h Thu Nov 8 00:32:36 2012 (r242726) +++ head/sys/fs/fuse/fuse_internal.h Thu Nov 8 00:32:49 2012 (r242727) @@ -138,23 +138,6 @@ uio_setresid(struct uio *uio, ssize_t re uio->uio_resid = resid; } -/* time */ - -#define fuse_timespec_add(vvp, uvp) \ - do { \ - (vvp)->tv_sec += (uvp)->tv_sec; \ - (vvp)->tv_nsec += (uvp)->tv_nsec; \ - if ((vvp)->tv_nsec >= 1000000000) { \ - (vvp)->tv_sec++; \ - (vvp)->tv_nsec -= 1000000000; \ - } \ - } while (0) - -#define fuse_timespec_cmp(tvp, uvp, cmp) \ - (((tvp)->tv_sec == (uvp)->tv_sec) ? \ - ((tvp)->tv_nsec cmp (uvp)->tv_nsec) : \ - ((tvp)->tv_sec cmp (uvp)->tv_sec)) - /* miscellaneous */ static __inline__ @@ -254,17 +237,9 @@ fuse_internal_attr_fat2vat(struct mount } -#define cache_attrs(vp, fuse_out) do { \ - struct timespec uptsp_ ## __func__; \ - \ - VTOFUD(vp)->cached_attrs_valid.tv_sec = (fuse_out)->attr_valid; \ - VTOFUD(vp)->cached_attrs_valid.tv_nsec = (fuse_out)->attr_valid_nsec; \ - nanouptime(&uptsp_ ## __func__); \ - \ - fuse_timespec_add(&VTOFUD(vp)->cached_attrs_valid, &uptsp_ ## __func__); \ - \ - fuse_internal_attr_fat2vat(vnode_mount(vp), &(fuse_out)->attr, VTOVA(vp)); \ -} while (0) +#define cache_attrs(vp, fuse_out) \ + fuse_internal_attr_fat2vat(vnode_mount(vp), &(fuse_out)->attr, \ + VTOVA(vp)); /* fsync */ Modified: head/sys/fs/fuse/fuse_io.c ============================================================================== --- head/sys/fs/fuse/fuse_io.c Thu Nov 8 00:32:36 2012 (r242726) +++ head/sys/fs/fuse/fuse_io.c Thu Nov 8 00:32:49 2012 (r242727) @@ -159,7 +159,6 @@ fuse_io_dispatch(struct vnode *vp, struc FS_DEBUG("direct write of vnode %ju via file handle %ju\n", (uintmax_t)VTOILLU(vp), (uintmax_t)fufh->fh_id); err = fuse_write_directbackend(vp, uio, cred, fufh); - fuse_invalidate_attr(vp); } else { FS_DEBUG("buffered write of vnode %ju\n", (uintmax_t)VTOILLU(vp)); Modified: head/sys/fs/fuse/fuse_node.c ============================================================================== --- head/sys/fs/fuse/fuse_node.c Thu Nov 8 00:32:36 2012 (r242726) +++ head/sys/fs/fuse/fuse_node.c Thu Nov 8 00:32:49 2012 (r242727) @@ -283,16 +283,6 @@ fuse_vnode_open(struct vnode *vp, int32_ } int -fuse_isvalid_attr(struct vnode *vp) -{ - struct fuse_vnode_data *fvdat = VTOFUD(vp); - struct timespec uptsp; - - nanouptime(&uptsp); - return fuse_timespec_cmp(&uptsp, &fvdat->cached_attrs_valid, <=); -} - -int fuse_vnode_savesize(struct vnode *vp, struct ucred *cred) { struct fuse_vnode_data *fvdat = VTOFUD(vp); @@ -337,8 +327,6 @@ fuse_vnode_savesize(struct vnode *vp, st if (err == 0) fvdat->flag &= ~FN_SIZECHANGE; - fuse_invalidate_attr(vp); - return err; } @@ -350,8 +338,7 @@ fuse_vnode_refreshsize(struct vnode *vp, struct vattr va; if ((fvdat->flag & FN_SIZECHANGE) != 0 || - (fuse_refresh_size == 0 && fvdat->filesize != 0) || - fuse_isvalid_attr(vp)) + (fuse_refresh_size == 0 && fvdat->filesize != 0)) return; VOP_GETATTR(vp, &va, cred); @@ -378,7 +365,5 @@ fuse_vnode_setsize(struct vnode *vp, str err = vtruncbuf(vp, cred, newsize, fuse_iosize(vp)); } vnode_pager_setsize(vp, newsize); - fuse_invalidate_attr(vp); - return err; } Modified: head/sys/fs/fuse/fuse_node.h ============================================================================== --- head/sys/fs/fuse/fuse_node.h Thu Nov 8 00:32:36 2012 (r242726) +++ head/sys/fs/fuse/fuse_node.h Thu Nov 8 00:32:49 2012 (r242727) @@ -83,7 +83,6 @@ struct fuse_vnode_data { uint32_t flag; /** meta **/ - struct timespec cached_attrs_valid; struct vattr cached_attrs; off_t filesize; uint64_t nlookup; @@ -100,15 +99,6 @@ struct fuse_vnode_data { extern struct vop_vector fuse_vnops; -static __inline__ -void -fuse_invalidate_attr(struct vnode *vp) -{ - if (VTOFUD(vp)) { - bzero(&VTOFUD(vp)->cached_attrs_valid, sizeof(struct timespec)); - } -} - static __inline void fuse_vnode_setparent(struct vnode *vp, struct vnode *dvp) { @@ -118,8 +108,6 @@ fuse_vnode_setparent(struct vnode *vp, s } } -int fuse_isvalid_attr(struct vnode *vp); - void fuse_vnode_destroy(struct vnode *vp); int fuse_vnode_get(struct mount *mp, Modified: head/sys/fs/fuse/fuse_vnops.c ============================================================================== --- head/sys/fs/fuse/fuse_vnops.c Thu Nov 8 00:32:36 2012 (r242726) +++ head/sys/fs/fuse/fuse_vnops.c Thu Nov 8 00:32:49 2012 (r242727) @@ -482,17 +482,6 @@ fuse_vnop_getattr(struct vop_getattr_arg /* Note that we are not bailing out on a dead file system just yet. */ - /* look for cached attributes */ - if (fuse_isvalid_attr(vp)) { - if (vap != VTOVA(vp)) { - memcpy(vap, VTOVA(vp), sizeof(*vap)); - } - if ((fvdat->flag & FN_SIZECHANGE) != 0) { - vap->va_size = fvdat->filesize; - } - debug_printf("return cached: inode=%ju\n", (uintmax_t)VTOI(vp)); - return 0; - } if (!(dataflags & FSESS_INITED)) { if (!vnode_isvroot(vp)) { fdata_set_dead(fuse_get_mpdata(vnode_mount(vp))); @@ -519,6 +508,11 @@ fuse_vnop_getattr(struct vop_getattr_arg if (vap != VTOVA(vp)) { memcpy(vap, VTOVA(vp), sizeof(*vap)); } + if (vap->va_type != vnode_vtype(vp)) { + fuse_internal_vnode_disappear(vp); + err = ENOENT; + goto out; + } if ((fvdat->flag & FN_SIZECHANGE) != 0) vap->va_size = fvdat->filesize; @@ -534,7 +528,6 @@ fuse_vnop_getattr(struct vop_getattr_arg fuse_vnode_setsize(vp, cred, new_filesize); } } - KASSERT(vnode_vtype(vp) == vap->va_type, ("stale vnode")); debug_printf("fuse_getattr e: returning 0\n"); out: @@ -635,9 +628,6 @@ fuse_vnop_link(struct vop_link_args *ap) feo = fdi.answ; err = fuse_internal_checkentry(feo, vnode_vtype(vp)); - fuse_invalidate_attr(tdvp); - fuse_invalidate_attr(vp); - out: fdisp_destroy(&fdi); return err; @@ -1085,8 +1075,6 @@ fuse_vnop_mkdir(struct vop_mkdir_args *a struct componentname *cnp = ap->a_cnp; struct vattr *vap = ap->a_vap; - int err = 0; - struct fuse_mkdir_in fmdi; fuse_trace_printf_vnop(); @@ -1096,13 +1084,8 @@ fuse_vnop_mkdir(struct vop_mkdir_args *a } fmdi.mode = MAKEIMODE(vap->va_type, vap->va_mode); - err = fuse_internal_newentry(dvp, vpp, cnp, FUSE_MKDIR, &fmdi, - sizeof(fmdi), VDIR); - - if (err == 0) { - fuse_invalidate_attr(dvp); - } - return err; + return (fuse_internal_newentry(dvp, vpp, cnp, FUSE_MKDIR, &fmdi, + sizeof(fmdi), VDIR)); } /* @@ -1367,10 +1350,8 @@ fuse_vnop_remove(struct vop_remove_args err = fuse_internal_remove(dvp, vp, cnp, FUSE_UNLINK); - if (err == 0) { + if (err == 0) fuse_internal_vnode_disappear(vp); - fuse_invalidate_attr(dvp); - } return err; } @@ -1423,11 +1404,8 @@ fuse_vnop_rename(struct vop_rename_args sx_xlock(&data->rename_lock); err = fuse_internal_rename(fdvp, fcnp, tdvp, tcnp); if (err == 0) { - fuse_invalidate_attr(fdvp); - if (tdvp != fdvp) { + if (tdvp != fdvp) fuse_vnode_setparent(fvp, tdvp); - fuse_invalidate_attr(tdvp); - } if (tvp != NULL) fuse_vnode_setparent(tvp, NULL); } @@ -1482,10 +1460,8 @@ fuse_vnop_rmdir(struct vop_rmdir_args *a } err = fuse_internal_remove(dvp, vp, ap->a_cnp, FUSE_RMDIR); - if (err == 0) { + if (err == 0) fuse_internal_vnode_disappear(vp); - fuse_invalidate_attr(dvp); - } return err; } @@ -1593,14 +1569,10 @@ fuse_vnop_setattr(struct vop_setattr_arg vap->va_vaflags & VA_UTIMES_NULL) { err = fuse_internal_access(vp, VWRITE, &facp, td, cred); } - if (err) { - fuse_invalidate_attr(vp); + if (err) goto out; - } - if ((err = fdisp_wait_answ(&fdi))) { - fuse_invalidate_attr(vp); + if ((err = fdisp_wait_answ(&fdi))) goto out; - } vtyp = IFTOVT(((struct fuse_attr_out *)fdi.answ)->attr.mode); if (vnode_vtype(vp) != vtyp) { @@ -1624,7 +1596,6 @@ fuse_vnop_setattr(struct vop_setattr_arg out: fdisp_destroy(&fdi); if (!err && sizechanged) { - fuse_invalidate_attr(vp); fuse_vnode_setsize(vp, cred, newsize); VTOFUD(vp)->flag &= ~FN_SIZECHANGE; } @@ -1715,10 +1686,6 @@ fuse_vnop_symlink(struct vop_symlink_arg err = fuse_internal_newentry_core(dvp, vpp, cnp, VLNK, &fdi); fdisp_destroy(&fdi); - - if (err == 0) { - fuse_invalidate_attr(dvp); - } return err; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201211080032.qA80Wo6Z039813>