Date: Wed, 11 Aug 2010 18:25:54 GMT From: Ilya Putsikau <ilya@skunkworks.freebsd.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 182127 for review Message-ID: <201008111825.o7BIPseu083893@skunkworks.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@182127?ac=10 Change 182127 by ilya@ilya_triton on 2010/08/09 12:29:53 Update tests, add inotify tests, add linux test output. Fix various bugs found with test cases. Don't save/lookup full paths use relative paths only, as linux does. Affected files ... .. //depot/projects/soc2010/ilya_fsnotify/src/sys/kern/vfs_notify.c#10 edit .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/fsnotify/regress.02.sh#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/fsnotify/regress.03.sh#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/fsnotify/regress.04.sh#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/Makefile#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/inotify-test#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/inotify-test.c#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/inotify-test.o#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.00.out#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.00.out-linux#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.00.sh#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.01.out#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.01.out-linux#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.01.sh#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.02.out#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.02.out-linux#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.02.sh#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.03.out#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.03.out-linux#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.03.sh#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.04.out#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.04.out-linux#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.04.sh#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.m4#1 add .. //depot/projects/soc2010/ilya_fsnotify/src/tools/regression/inotify/regress.sh#1 add Differences ... ==== //depot/projects/soc2010/ilya_fsnotify/src/sys/kern/vfs_notify.c#10 (text+ko) ==== @@ -76,11 +76,10 @@ struct mtx nd_mtx; struct vnode *nd_vnode; struct mount *nd_mount; - char *nd_path; - char *nd_pathfree; + char *nd_name; ino_t nd_ino; volatile u_int nd_refcnt; - int nd_pathlen; + int nd_namelen; int nd_watchcount; u_int nd_supermask; int nd_flags; @@ -95,8 +94,8 @@ struct fnevent { TAILQ_ENTRY(fnevent) ev_queueentry; struct fnnode *ev_node; - char *ev_pathfree; - int ev_pathpos; + char *ev_name; + int ev_namelen; int ev_mask; int ev_cookie; int ev_handlecount; @@ -182,7 +181,8 @@ int mask, struct fnwatch **watchpp); static int session_rmwatch(struct fnsession *ss, int wd); -static struct fnnode* node_alloc(struct vnode *vp, ino_t ino); +static struct fnnode* node_alloc(struct vnode *vp, ino_t ino, char *name, + int namelen); static struct fnnode* node_lookup(struct vnode *vp); static struct fnnode* node_lookupex(struct vnode *vp, ino_t *inop, int flags); static void node_hold(struct fnnode *node); @@ -190,15 +190,12 @@ static void node_watchhold(struct fnnode *node); static void node_watchdrop(struct fnnode *node); -static void event_copypath(struct fnevent *event, char *path); -static int event_userpathlen(struct fnevent *event); static void event_enqueue(struct fnnode *node, struct componentname *cnp, int *cookiep, int mask); static void watch_free(struct fnwatch *watch); -#define NODE_ISDIR 0x0001 -#define NODE_CHANGED 0x0002 +#define NODE_CHANGED 0x0001 #define LOOKUP_VPLOCKED 0x0001 #define LOOKUP_IGNINVAL 0x0002 @@ -339,6 +336,7 @@ while (!TAILQ_EMPTY(&ss->ss_watchlist)) { watch = TAILQ_FIRST(&ss->ss_watchlist); watch_free(watch); + SESSION_LOCK(ss); } SESSION_UNLOCK(ss); @@ -378,7 +376,7 @@ struct fnwatch *watch; struct fsnotify_event *fe; int destroy, len, error; - char user_buf[sizeof(struct fsnotify_event) + MAXPATHLEN]; + char user_buf[sizeof(struct fsnotify_event) + NAME_MAX + 1]; printf("fsnotify_read: offset %jd\n", uio->uio_offset); @@ -413,24 +411,24 @@ } event = eh->eh_event; watch = eh->eh_watch; - fe->len = event_userpathlen(event); + fe->len = event->ev_namelen + 1; len = fe->len + sizeof(struct fsnotify_event); if (len > uio->uio_resid) { SESSION_UNLOCK(ss); break; } fe->wd = watch->wt_wd; - fe->mask = watch->wt_mask & event->ev_mask; + fe->mask = (FN_FLAGS | watch->wt_mask) & event->ev_mask; fe->fileno = event->ev_node->nd_ino; fe->cookie = event->ev_cookie; - event_copypath(event, fe->name); + memcpy(fe->name, event->ev_name, event->ev_namelen + 1); destroy = event->ev_mask & FN_DESTROY; session_drophandle(ss, eh); if (destroy != 0) watch_free(watch); - - SESSION_UNLOCK(ss); + else + SESSION_UNLOCK(ss); MPASS(len <= uio->uio_resid); error = uiomove(user_buf, len, uio); @@ -457,7 +455,7 @@ SESSION_LOCK(ss); TAILQ_FOREACH(eh, &ss->ss_queue, eh_queueentry) { *nread += sizeof(struct fsnotify_event) + - event_userpathlen(eh->eh_event); + eh->eh_event->ev_namelen; } SESSION_UNLOCK(ss); @@ -472,21 +470,24 @@ struct fnwatch *watch; struct file *fp; struct filedesc *fdp; - struct vnode *vp; - char *path, *pathfree; + struct vnode *vp, *xvp; + char *name; ino_t ino; int error = 0, vfslocked; + u_int namelen; fdp = td->td_proc->p_fd; vp = NULL; FILEDESC_SLOCK(fdp); fp = fget_locked(fdp, ap->fd); - if (fp != NULL && fp->f_type == DTYPE_VNODE) { + if (fp != NULL && fp->f_type == DTYPE_VNODE) vp = fp->f_vnode; - } FILEDESC_SUNLOCK(fdp); if (vp == NULL) return (EBADF); + /* FIXME FIXME */ + if (vp->v_type != VDIR) + return (EINVAL); vfslocked = VFS_LOCK_GIANT(vp->v_mount); ino = 0; node = node_lookupex(vp, &ino, LOOKUP_IGNINVAL); @@ -495,16 +496,21 @@ NODE_UNLOCK(node); VFS_UNLOCK_GIANT(vfslocked); } else { - error = vn_fullpath_global(td, vp, &path, &pathfree); - VFS_UNLOCK_GIANT(vfslocked); + namelen = NAME_MAX; + name = malloc(namelen + 1, M_FSNOTIFY, M_WAITOK); + xvp = vp; + error = vn_vptocnp(&xvp, td->td_ucred, name, &namelen); if (error == 0) { - node = node_alloc(vp, ino); - node->nd_path = path; - node->nd_pathlen = strlen(path); - node->nd_pathfree = pathfree; + memcpy(name, name + namelen, NAME_MAX - namelen); + namelen = NAME_MAX - namelen; + name[namelen] = '\0'; + node = node_alloc(vp, ino, name, namelen); node_watchhold(node); + vdrop(xvp); NODE_UNLOCK(node); - } + } else + free(name, M_FSNOTIFY); + VFS_UNLOCK_GIANT(vfslocked); } if ((ap->mask & FN_CLOSEFD) != 0) { @@ -580,6 +586,13 @@ /* * VFS hooks */ + +static __inline int +vp_isdir(struct vnode *vp) +{ + return (vp->v_type == VDIR ? FN_ISDIR : 0); +} + static void hook_reclaim(struct vnode *vp) { @@ -604,12 +617,13 @@ static void hook_open(struct vop_open_args *ap) { + struct vnode *vp = ap->a_vp; struct fnnode *node; int cookie = 0; - node = node_lookup(ap->a_vp); + node = node_lookup(vp); if (node != NULL) { - if ((node->nd_flags & NODE_ISDIR) == 0) + if (vp_isdir(vp) == 0) event_enqueue(node, NULL, &cookie, FN_OPEN); else NODE_UNLOCK(node); @@ -619,12 +633,13 @@ static void hook_close(struct vop_close_args *ap) { + struct vnode *vp = ap->a_vp; struct fnnode *node; int cookie = 0, mask; - node = node_lookup(ap->a_vp); + node = node_lookup(vp); if (node != NULL) { - if ((node->nd_flags & NODE_ISDIR) == 0) { + if (vp_isdir(vp) == 0) { mask = (node->nd_flags & NODE_CHANGED) != 0 ? FN_CLOSE_RW : FN_CLOSE_RO; event_enqueue(node, NULL, &cookie, mask); @@ -652,11 +667,12 @@ static void hook_write(struct vop_write_args *ap) { + struct vnode *vp = ap->a_vp; struct fnnode *node; - node = node_lookup(ap->a_vp); + node = node_lookup(vp); if (node != NULL) { - if ((node->nd_flags & NODE_ISDIR) == 0) + if (vp_isdir(vp) == 0) node->nd_flags |= NODE_CHANGED; NODE_UNLOCK(node); } @@ -665,12 +681,14 @@ static void hook_setattr(struct vop_setattr_args *ap) { + struct vnode *vp = ap->a_vp; struct fnnode *node; int cookie = 0; - node = node_lookup(ap->a_vp); + node = node_lookup(vp); if (node != NULL) { - event_enqueue(node, NULL, &cookie, FN_SETATTR); + event_enqueue(node, NULL, &cookie, + FN_SETATTR | vp_isdir(vp)); } } @@ -683,7 +701,8 @@ dirnode = node_lookup(dvp); if (dirnode != NULL) - event_enqueue(dirnode, cnp, &cookie, FN_CREATE); + event_enqueue(dirnode, cnp, &cookie, + FN_CREATE | vp_isdir(vp)); return (0); } @@ -698,11 +717,13 @@ node = node_lookup(vp); if (node != NULL) - event_enqueue(node, NULL, &cookie, FN_DESTROY | FN_REMOVE); + event_enqueue(node, NULL, &cookie, + FN_DESTROY | FN_REMOVE | vp_isdir(vp)); dirnode = node_lookup(dvp); if (dirnode != NULL) - event_enqueue(dirnode, cnp, &cookie, FN_REMOVE); + event_enqueue(dirnode, cnp, &cookie, + FN_REMOVE | vp_isdir(vp)); } static void @@ -749,13 +770,16 @@ hook_rename(struct vop_rename_args *ap) { struct fnnode *fdirnode, *fnode, *tdirnode, *tnode; + int dirmask; int cookie = 0; + dirmask = vp_isdir(ap->a_fvp); + if (ap->a_tvp != NULL) { tnode = node_lookupex(ap->a_tvp, NULL, 0); if (tnode != NULL) { event_enqueue(tnode, NULL, &cookie, - FN_DESTROY | FN_REMOVE); + FN_DESTROY | FN_REMOVE | dirmask); } } fnode = node_lookupex(ap->a_fvp, NULL, 0); @@ -767,10 +791,12 @@ fdirnode = node_lookupex(ap->a_fdvp, NULL, 0); if (fdirnode != NULL) - event_enqueue(fdirnode, ap->a_fcnp, &cookie, FN_RENAME_FROM); + event_enqueue(fdirnode, ap->a_fcnp, &cookie, + FN_RENAME_FROM | dirmask); tdirnode = node_lookupex(ap->a_tdvp, NULL, 0); if (tdirnode != NULL) - event_enqueue(tdirnode, ap->a_tcnp, &cookie, FN_RENAME_TO); + event_enqueue(tdirnode, ap->a_tcnp, &cookie, + FN_RENAME_TO | dirmask); } static void @@ -810,14 +836,15 @@ TAILQ_REMOVE(&ss->ss_watchlist, watch, wt_sessionentry); watch->wt_session = NULL; - free_unr(fsnotify_wds, watch->wt_wd); TAILQ_FOREACH(eh, &ss->ss_queue, eh_queueentry) { MPASS(eh->eh_watch != watch); } - MPASS(watch->wt_session == NULL && watch->wt_node == NULL); + MPASS(watch->wt_node == NULL); printf("watch_free: free %p\n", watch); + SESSION_UNLOCK(ss); + free_unr(fsnotify_wds, watch->wt_wd); free(watch, M_FSNOTIFY); } @@ -865,7 +892,7 @@ INOHASH_UNLOCK(); } mtx_destroy(&node->nd_mtx); - free(node->nd_pathfree, M_TEMP); + free(node->nd_name, M_FSNOTIFY); free(node, M_FSNOTIFY); } } @@ -880,7 +907,7 @@ } static struct fnnode * -node_alloc(struct vnode *vp, ino_t ino) +node_alloc(struct vnode *vp, ino_t ino, char *path, int namelen) { struct fnnode *node; @@ -895,6 +922,8 @@ TAILQ_INIT(&node->nd_watchlist); node->nd_ino = ino; + node->nd_name = path; + node->nd_namelen = namelen; NODE_LOCK(node); @@ -907,8 +936,6 @@ vholdl(vp); node->nd_vnode = vp; vp->v_fnnode = node; - if (vp->v_type == VDIR) - node->nd_flags |= NODE_ISDIR; VI_UNLOCK(vp); return (node); @@ -1038,95 +1065,27 @@ return (node_lookupex(vp, NULL, LOOKUP_VPLOCKED)); } -static int -node_updatepath(struct fnnode *node) -{ - struct nameidata ni; - struct vnode *vp; - char *path, *pathfree; - char *npath, *npathfree; - int vfslocked, error; - - /* Should be executed in *single* fsnotify_daemon thread */ - vp = node->nd_vnode; - if ((vp->v_iflag & VI_DOOMED) != 0 || vp->v_usecount == 0) { - printf("skip node path update: %p\n", vp); - return (ENOENT); - } - - printf("node_updatepath: node %p vp %p %s\n", - node, vp, node->nd_path); - path = node->nd_path; - pathfree = node->nd_pathfree; - npath = npathfree = NULL; - vhold(vp); - NODE_UNLOCK(node); - - vref(rootvnode); - NDINIT_ATVP(&ni, LOOKUP, MPSAFE | FOLLOW, UIO_SYSSPACE, path, rootvnode, - curthread); - error = namei(&ni); - if (error == 0) { - if (vp != ni.ni_vp) { - printf("fsnotify: vnode was replaced between lookups: %s\n", - path); - error = ENOENT; - } - NDFREE(&ni, 0); - } - if (error != 0) { - vfslocked = VFS_LOCK_GIANT(vp->v_mount); - error = vn_fullpath_global(curthread, vp, &npath, &npathfree); - VFS_UNLOCK_GIANT(vfslocked); - } - - if ((vp->v_iflag & VI_DOOMED) != 0) { - printf("fsnotify: vnode is doomed: %s\n", path); - error = ENOENT; - } - vdrop(vp); - NODE_LOCK(node); - if (path != node->nd_path) { - /* Lookup race */ - free(pathfree, M_TEMP); - MPASS(node->nd_path != NULL); - error = 0; - } else if (error == 0 && npath != NULL) { - free(node->nd_pathfree, M_TEMP); - node->nd_path = npath; - node->nd_pathlen = strlen(npath); - node->nd_pathfree = npathfree; - npathfree = NULL; - } - if (npathfree != NULL) - free(npathfree, M_TEMP); - return (error); -} - static struct fnevent * event_alloc(struct fnnode *node, char *name, int namelen, int handle_maxsize, int mask, int cookie) { struct fnevent *event; + MPASS(namelen > 0 && name != NULL); MPASS(handle_maxsize > 0); MPASS(mask != 0); event = malloc(sizeof(struct fnevent) + - (sizeof(struct fneventhandle) * handle_maxsize), + (sizeof(struct fneventhandle) * handle_maxsize) + namelen + 1, M_FSNOTIFY, M_WAITOK | M_ZERO); event->ev_handlemaxsize = handle_maxsize; event->ev_node = node; event->ev_mask = mask; event->ev_cookie = cookie; - event->ev_pathfree = uma_zalloc(namei_zone, M_WAITOK); - event->ev_pathpos = MAXPATHLEN - 1 - namelen; - event->ev_pathfree[MAXPATHLEN - 1] = '\0'; - if (name != NULL) { - MPASS((node->nd_flags & NODE_ISDIR) != 0); - memcpy(event->ev_pathfree + event->ev_pathpos, name, namelen); - } - printf("event alloc: %p\n", event); + event->ev_namelen = namelen; + event->ev_name = (char *)(event->ev_handlebuf + handle_maxsize); + memcpy(event->ev_name, name, namelen); + printf("event alloc: %s\n", event->ev_name); return (event); } @@ -1134,32 +1093,11 @@ static void event_free(struct fnevent *event) { + printf("event free: %s\n", event->ev_name); node_drop(event->ev_node); - uma_zfree(namei_zone, event->ev_pathfree); free(event, M_FSNOTIFY); - printf("event free: %p\n", event); -} - -static __inline int -event_pathlen(struct fnevent *event) -{ - return (MAXPATHLEN - 1 - event->ev_pathpos); } -static __inline int -event_userpathlen(struct fnevent *event) -{ - /* Count last zero byte */ - return event_pathlen(event) + 1; -} - -static __inline void -event_copypath(struct fnevent *event, char *path) -{ - memcpy(path, event->ev_pathfree + event->ev_pathpos, - event_userpathlen(event)); -} - static void eventhandle_drop(struct fneventhandle *eh) { @@ -1176,24 +1114,6 @@ event_free(eh->eh_event); } -static void -event_prependpath(struct fnevent *event, struct fnnode *node) -{ - int pos, len; - - pos = event->ev_pathpos; - len = node->nd_pathlen; - MPASS(len > 0 && node->nd_path[len - 1] != '/'); - MPASS(pos >= len + 1); - - if (event_pathlen(event) != 0) - event->ev_pathfree[--pos] = '/'; - pos -= len; - memcpy(event->ev_pathfree + pos, node->nd_path, len); - - event->ev_pathpos = pos; -} - static int event_nextcookie(void) { @@ -1240,6 +1160,7 @@ if (*cookiep == 0) *cookiep = event_nextcookie(); + event = event_alloc(node, nameptr, namelen, watchcount + 1, mask, *cookiep); @@ -1258,13 +1179,14 @@ struct fnwatch *w; mask &= ~FN_FLAGS_INTERNAL; - if ((mask & FN_FLAGS) != 0) { - printf("fsnotify: invalid watch mask: %08x\n", mask); - mask &= ~FN_FLAGS; + if ((mask & ~FN_ALL) != 0) { + printf("fsnotify: invalid watch mask: %08x -> %08x\n", + mask, mask & FN_ALL); + mask &= FN_ALL; } printf("session_addwatch: %s: node %p session %p\n", - node->nd_path, node, ss); + node->nd_name, node, ss); watch = malloc(sizeof(struct fnwatch), M_FSNOTIFY, M_WAITOK | M_ZERO); printf("watch alloc: %p\n", watch); @@ -1411,13 +1333,6 @@ event_free(event); continue; } - if (node->nd_vnode != NULL && - (event->ev_mask & FN_DESTROY) == 0) - node_updatepath(node); - else - printf("fsnotify: vnode not found, reusing cached path: %s\n", - node->nd_path); - event_prependpath(event, node); if (event->ev_mask & FN_DESTROY) node_detachallwatches(node);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201008111825.o7BIPseu083893>