Date: Mon, 21 Jun 2010 08:27:15 GMT From: Ilya Putsikau <ilya@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 180031 for review Message-ID: <201006210827.o5L8RFDi072816@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@180031?ac=10 Change 180031 by ilya@ilya_triton on 2010/06/21 08:26:19 Embed fnnode reference to vnode Add reclaim hook Remove vp hash, use only inode hash table Extend node_lookupex to ignore invalid node reference and optionally lock vnode Fix vnode locking in hook_rename Affected files ... .. //depot/projects/soc2010/ilya_fsnotify/src/sys/kern/vfs_notify.c#4 edit .. //depot/projects/soc2010/ilya_fsnotify/src/sys/kern/vfs_subr.c#3 edit .. //depot/projects/soc2010/ilya_fsnotify/src/sys/sys/fsnotify.h#4 edit .. //depot/projects/soc2010/ilya_fsnotify/src/sys/sys/vnode.h#2 edit Differences ... ==== //depot/projects/soc2010/ilya_fsnotify/src/sys/kern/vfs_notify.c#4 (text+ko) ==== @@ -123,8 +123,7 @@ static struct task fsnotify_task; static struct taskqueue *fsnotify_tq; -static struct fnnode_hashhead **fnnode_vphashtbl; -static struct fnnode_hashhead **fnnode_inohashtbl; +static struct fnnode_hashhead *fnnode_inohashtbl; static struct mtx fnnode_hashmtx; static u_long fnnode_hashmask; @@ -142,6 +141,7 @@ .d_name = "fsnotify", }; +static void hook_reclaim(struct vnode *vp); static vop_create_t hook_create; static vop_link_t hook_link; static vop_mkdir_t hook_mkdir; @@ -160,13 +160,18 @@ static int session_rmwatch(struct fnsession *ss, int wd); static struct fnnode* node_lookup(struct vnode *vp); static struct fnnode* node_lookupex(struct vnode *vp, ino_t *inop, - int vplocked); + int flags); static struct fnnode* node_alloc(struct vnode *vp, ino_t ino); -static void node_destroy(struct fnnode *node); +static void node_drop(struct fnnode *node); static void event_copypath(struct fnevent *event, char *path, int *pathlen); static int event_pathlen(struct fnevent *event); static int event_nextcookie(void); +#define NODE_ISVALID(a) ((a) != NULL && (a) != FNNODE_INVAL) + +#define LOOKUP_VPLOCKED 0x0001 +#define LOOKUP_IGNINVAL 0x0002 + static int fsnotify_modevent(struct module *module, int cmd, void *arg) { @@ -175,7 +180,8 @@ switch (cmd) { case MOD_LOAD: - if (fsnotify_hook_create != NULL || + if (fsnotify_hook_reclaim != NULL || + fsnotify_hook_create != NULL || fsnotify_hook_link != NULL || fsnotify_hook_mkdir != NULL || fsnotify_hook_remove != NULL || @@ -190,8 +196,6 @@ mtx_init(&fnnode_hashmtx, "fsnotify_hash", NULL, MTX_DEF); hashsize = MAX(desiredvnodes / 32, 16); - fnnode_vphashtbl = hashinit(hashsize, M_FSNOTIFYHASH, - &fnnode_hashmask); fnnode_inohashtbl = hashinit(hashsize, M_FSNOTIFYHASH, &fnnode_hashmask); @@ -205,6 +209,7 @@ fsnotify_dev = make_dev(&fsnotify_cdevsw, 0, UID_ROOT, GID_WHEEL, 0666, "fsnotify"); + fsnotify_hook_reclaim = hook_reclaim; fsnotify_hook_create = hook_create; fsnotify_hook_link = hook_link; fsnotify_hook_mkdir = hook_mkdir; @@ -214,6 +219,7 @@ fsnotify_hook_symlink = hook_symlink; break; case MOD_UNLOAD: + fsnotify_hook_reclaim = NULL; fsnotify_hook_create = NULL; fsnotify_hook_link = NULL; fsnotify_hook_mkdir = NULL; @@ -224,7 +230,6 @@ destroy_dev(fsnotify_dev); taskqueue_drain(fsnotify_tq, &fsnotify_task); taskqueue_free(fsnotify_tq); - free(fnnode_vphashtbl, M_FSNOTIFYHASH); free(fnnode_inohashtbl, M_FSNOTIFYHASH); mtx_destroy(&fsnotify_queue_mtx); mtx_destroy(&fnnode_hashmtx); @@ -279,6 +284,8 @@ mtx_init(&ss->ss_mtx, "fnsession_queue", NULL, MTX_DEF); cv_init(&ss->ss_queuecv, "fnsession_queuecv"); + TAILQ_INIT(&ss->ss_queue); + TAILQ_INIT(&ss->ss_watchlist); devfs_set_cdevpriv(ss, fsnotify_session_dtor); @@ -353,7 +360,6 @@ { struct fnsession *ss; struct fnnode *node; - struct fnnode *freenode; struct fnevent *event; struct fnwatch *watch; struct fsnotify_addwatch_args *add_args; @@ -383,10 +389,9 @@ if (vp == NULL) return (EBADF); vfslocked = VFS_LOCK_GIANT(vp->v_mount); - node = node_lookupex(vp, &ino, 0); + node = node_lookupex(vp, &ino, LOOKUP_IGNINVAL); if (node != NULL) { VFS_UNLOCK_GIANT(vfslocked); - freenode = NULL; } else { error = vn_fullpath(td, vp, &path, &pathfree); VFS_UNLOCK_GIANT(vfslocked); @@ -396,14 +401,11 @@ node->nd_path = path; node->nd_pathlen = strlen(path); node->nd_pathfree = pathfree; - freenode = node; } error = session_addwatch(ss, node, add_args->fa_mask, &watch); - if (error == 0) { + if (error == 0) add_args->fa_wd = watch->wt_wd; - } else if (freenode != NULL) { - node_destroy(node); - } + /* If error != 0 node will be removed by hook_reclaim */ break; case FSNOTIFY_RMWATCH: error = session_rmwatch(ss, *(int *)data); @@ -453,6 +455,20 @@ /* * VFS hooks */ +static void +hook_reclaim(struct vnode *vp) +{ + struct fnnode *node; + + VI_LOCK(vp); + node = vp->v_fnnode; + vp->v_fnnode = NULL; + VI_UNLOCK(vp); + + if (NODE_ISVALID(node)) + node_drop(node); +} + static __inline int hook_generic_create(struct vnode *dvp, struct vnode *vp, struct componentname *cnp) @@ -533,7 +549,7 @@ cookie = event_nextcookie(); if (ap->a_tvp != NULL) { - tnode = node_lookup(ap->a_tvp); + tnode = node_lookupex(ap->a_tvp, NULL, 0); if (tnode != NULL) { enqueue_fileevent(tnode, cookie, FE_DESTROY | FE_REMOVE); @@ -548,7 +564,7 @@ fdirnode = node_lookupex(ap->a_fdvp, NULL, 0); if (fdirnode != NULL) enqueue_direvent(fdirnode, ap->a_fcnp, cookie, FE_RENAME_FROM); - tdirnode = node_lookup(ap->a_tdvp); + tdirnode = node_lookupex(ap->a_tdvp, NULL, 0); if (tdirnode != NULL) enqueue_direvent(tdirnode, ap->a_tcnp, cookie, FE_RENAME_TO); @@ -591,6 +607,11 @@ if (refcount_release(&node->nd_refcnt) != 0) { KASSERT(node->nd_watchcount == 0 && node->nd_supermask == 0 && TAILQ_EMPTY(&node->nd_watchlist), ("Invalid reference count")); + if (node->nd_ino != 0) { + mtx_lock(&fnnode_hashmtx); + LIST_REMOVE(node, nd_hashentry); + mtx_unlock(&fnnode_hashmtx); + } mtx_destroy(&node->nd_mtx); free(node->nd_path, M_TEMP); free(node, M_FSNOTIFY); @@ -598,22 +619,12 @@ } static __inline struct fnnode_hashhead * -node_vphashhead(struct vnode *vp) -{ - uint32_t h; - - h = hash32_buf(vp, sizeof(vp), HASHINIT); - h += vp->v_mount->mnt_hashseed; - return fnnode_vphashtbl[h & fnnode_hashmask]; -} - -static __inline struct fnnode_hashhead * node_inohashhead(struct mount *mnt, ino_t ino) { uint32_t h; h = ino + mnt->mnt_hashseed; - return fnnode_inohashtbl[h & fnnode_hashmask]; + return &fnnode_inohashtbl[h & fnnode_hashmask]; } static struct fnnode * @@ -622,21 +633,19 @@ struct fnnode *node; MPASS(vp != NULL); + MPASS(ino != 0); node = malloc(sizeof(struct fnnode), M_FSNOTIFY, M_WAITOK | M_ZERO); refcount_init(&node->nd_refcnt, 1); + mtx_init(&node->nd_mtx, "fsnotify_node", NULL, MTX_DEF); + TAILQ_INIT(&node->nd_watchlist); + + node->nd_ino = ino; - mtx_lock(&fnnode_hashmtx); - /* DEBUG */ - LIST_FOREACH(node, node_vphashhead(vp), nd_hashentry) { - if (node->nd_vnode == vp) { - panic("Node already exists in vnode hash table: %p", - node->nd_vnode); - } - } - LIST_INSERT_HEAD(node_vphashhead(vp), node, nd_hashentry); - mtx_unlock(&fnnode_hashmtx); + VI_LOCK(vp); + vp->v_fnnode = node; + VI_UNLOCK(vp); return (node); } @@ -657,18 +666,6 @@ } } -static void -node_destroy(struct fnnode *node) -{ - mtx_lock(&node->nd_mtx); - node_detachwatches(node); - mtx_unlock(&node->nd_mtx); - mtx_lock(&fnnode_hashmtx); - LIST_REMOVE(node, nd_hashentry); - mtx_unlock(&fnnode_hashmtx); - node_drop(node); -} - static int node_getino(struct vnode *vp, ino_t *inop, int vplocked) { @@ -681,7 +678,10 @@ return (error); } - error = VOP_GETATTR(vp, &va, thread0.td_ucred); + if (vp->v_iflag & VI_DOOMED) + return (EINVAL); + + error = VOP_GETATTR(vp, &va, NOCRED); if (error == 0) *inop = va.va_fileid; @@ -692,51 +692,66 @@ } static struct fnnode* -node_lookupex(struct vnode *vp, ino_t *inop, int vplocked) +node_lookupex(struct vnode *vp, ino_t *inop, int flags) { struct fnnode *node, *rv; ino_t ino; - int error; + int error, watchcount; rv = NULL; - /* Node is always on one of the hash tables. */ - mtx_lock(&fnnode_hashmtx); - LIST_FOREACH(node, node_vphashhead(vp), nd_hashentry) { - if (node->nd_vnode == vp) { - mtx_lock(&node->nd_mtx); - rv = (node->nd_watchcount == 0 ? NULL : node); - if (rv == NULL) - mtx_unlock(&node->nd_mtx); - break; - } + VI_LOCK(vp); + node = vp->v_fnnode; + VI_UNLOCK(vp); + if (node == FNNODE_INVAL) { + node = NULL; + if ((flags & LOOKUP_IGNINVAL) == 0) + goto done; + } else if (node != NULL) { + mtx_lock(&node->nd_mtx); + watchcount = node->nd_watchcount; + mtx_unlock(&node->nd_mtx); + if (watchcount == 0) + node = NULL; + goto done; } - mtx_unlock(&fnnode_hashmtx); - if (node == NULL) { - if (inop == NULL) - inop = &ino; - error = node_getino(vp, inop, vplocked); - if (error != 0) - goto done; + node = NULL; + if (inop == NULL) + inop = &ino; + error = node_getino(vp, inop, flags & LOOKUP_VPLOCKED); + if (error != 0) + goto done; - mtx_lock(&fnnode_hashmtx); - LIST_FOREACH(node, - node_inohashhead(vp->v_mount, *inop), nd_hashentry) { - if (node->nd_ino != *inop || - node->nd_mount != vp->v_mount) - continue; - /* add to inohash */ - mtx_lock(&node->nd_mtx); - rv = (node->nd_watchcount == 0 ? NULL : node); - if (rv == NULL) - mtx_unlock(&node->nd_mtx); - break; - } + mtx_lock(&fnnode_hashmtx); + LIST_FOREACH(node, + node_inohashhead(vp->v_mount, *inop), nd_hashentry) { + if (node->nd_ino != *inop || + node->nd_mount != vp->v_mount) + continue; + mtx_lock(&node->nd_mtx); + VI_LOCK(vp); + if (!NODE_ISVALID(vp->v_fnnode)) { + node_hold(node); + vp->v_fnnode = node; + } else + MPASS(vp->v_fnnode == node); + VI_UNLOCK(vp); + watchcount = node->nd_watchcount; + mtx_unlock(&node->nd_mtx); + if (watchcount == 0) + node = NULL; mtx_unlock(&fnnode_hashmtx); + goto done; } + mtx_unlock(&fnnode_hashmtx); + VI_LOCK(vp); + if (!NODE_ISVALID(vp->v_fnnode)) + vp->v_fnnode = FNNODE_INVAL; + VI_UNLOCK(vp); + done: - return (rv); + return (node); } static __inline struct fnnode* @@ -744,7 +759,7 @@ { ASSERT_VOP_LOCKED(vp, "fsnotify node lookup"); - return (node_lookupex(vp, NULL, 1)); + return (node_lookupex(vp, NULL, LOOKUP_VPLOCKED)); } static int ==== //depot/projects/soc2010/ilya_fsnotify/src/sys/kern/vfs_subr.c#3 (text+ko) ==== @@ -275,6 +275,8 @@ /* * fsnotify hooks */ + +void (*fsnotify_hook_reclaim)(struct vnode *vp) = NULL; vop_create_t *fsnotify_hook_create = NULL; vop_link_t *fsnotify_hook_link = NULL; vop_mkdir_t *fsnotify_hook_mkdir = NULL; @@ -2598,6 +2600,8 @@ */ delmntque(vp); cache_purge(vp); + if (fsnotify_hook_reclaim != NULL) + fsnotify_hook_reclaim(vp); /* * Done with purge, reset to the standard lock and invalidate * the vnode. ==== //depot/projects/soc2010/ilya_fsnotify/src/sys/sys/fsnotify.h#4 (text+ko) ==== @@ -27,22 +27,22 @@ * $FreeBSD$ */ -#ifndef _SYS_FSNOTIFY_H_ +#ifndef _SYS_FSNOTIFY_H_ #define _SYS_FSNOTIFY_H_ -#define FE_CREATE 0x0001 -#define FE_REMOVE 0x0002 -#define FE_RENAME_FROM 0x0004 -#define FE_RENAME_TO 0x0008 +#define FE_CREATE 0x0001 +#define FE_REMOVE 0x0002 +#define FE_RENAME_FROM 0x0004 +#define FE_RENAME_TO 0x0008 -#define FE_CLOSE 0x0010 -#define FE_INACTIVE 0x0020 -#define FE_INACTIVE_CHANGED 0x0040 +#define FE_CLOSE 0x0010 +#define FE_INACTIVE 0x0020 +#define FE_INACTIVE_CHANGED 0x0040 -#define FE_DESTROY 0x0080 +#define FE_DESTROY 0x0080 -#define FSNOTIFY_ADDWATCH _IOWR('F', 1, struct fsnotify_addwatch_args) -#define FSNOTIFY_RMWATCH _IOW('F', 2, int) +#define FSNOTIFY_ADDWATCH _IOWR('F', 1, struct fsnotify_addwatch_args) +#define FSNOTIFY_RMWATCH _IOW('F', 2, int) struct fsnotify_event { int32_t fe_wd; @@ -61,6 +61,10 @@ #ifdef _KERNEL +#define FNNODE_INVAL ((void *)(uintptr_t)(-1)) + +extern void (*fsnotify_hook_reclaim)(struct vnode *vp); + extern vop_create_t *fsnotify_hook_create; extern vop_link_t *fsnotify_hook_link; extern vop_mkdir_t *fsnotify_hook_mkdir; @@ -69,7 +73,7 @@ extern vop_rmdir_t *fsnotify_hook_rmdir; extern vop_symlink_t *fsnotify_hook_symlink; -#endif /* _KERNEL */ +#endif /* _KERNEL */ -#endif /* !_SYS_FSNOTIFY_H_ */ +#endif /* !_SYS_FSNOTIFY_H_ */ ==== //depot/projects/soc2010/ilya_fsnotify/src/sys/sys/vnode.h#2 (text+ko) ==== @@ -60,6 +60,7 @@ * it from v_data. If non-null, this area is freed in getnewvnode(). */ +struct fnnode; struct namecache; struct vpollinfo { @@ -169,6 +170,7 @@ struct vpollinfo *v_pollinfo; /* G Poll events, p for *v_pi */ struct label *v_label; /* MAC label for vnode */ struct lockf *v_lockf; /* Byte-level lock list */ + struct fnnode *v_fnnode; /* i fsnotify node */ }; #endif /* defined(_KERNEL) || defined(_KVM_VNODE) */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201006210827.o5L8RFDi072816>