PERFORCE change 180499 for review
Ilya Putsikau
ilya at FreeBSD.org
Mon Jul 5 20:06:13 UTC 2010
http://p4web.freebsd.org/@@180499?ac=10
Change 180499 by ilya at ilya_triton on 2010/07/05 20:05:57
Fix bugs related to reference counting and resource deallocation
Affected files ...
.. //depot/projects/soc2010/ilya_fsnotify/src/sys/kern/vfs_notify.c#5 edit
Differences ...
==== //depot/projects/soc2010/ilya_fsnotify/src/sys/kern/vfs_notify.c#5 (text+ko) ====
@@ -162,10 +162,14 @@
static struct fnnode* node_lookupex(struct vnode *vp, ino_t *inop,
int flags);
static struct fnnode* node_alloc(struct vnode *vp, ino_t ino);
+static void node_hold(struct fnnode *node);
static void node_drop(struct fnnode *node);
+static void node_watchhold(struct fnnode *node);
+static void node_watchdrop(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);
+static void watch_free(struct fnwatch *watch);
#define NODE_ISVALID(a) ((a) != NULL && (a) != FNNODE_INVAL)
@@ -175,7 +179,9 @@
static int
fsnotify_modevent(struct module *module, int cmd, void *arg)
{
- int hashsize;
+ struct fnnode_hashhead *hashhead;
+ struct fnnode *node;
+ int i;
int error = 0;
switch (cmd) {
@@ -195,8 +201,8 @@
mtx_init(&fsnotify_queue_mtx, "fsnotify_queue", NULL, MTX_DEF);
mtx_init(&fnnode_hashmtx, "fsnotify_hash", NULL, MTX_DEF);
- hashsize = MAX(desiredvnodes / 32, 16);
- fnnode_inohashtbl = hashinit(hashsize, M_FSNOTIFYHASH,
+ i = MAX(desiredvnodes / 32, 16);
+ fnnode_inohashtbl = hashinit(i, M_FSNOTIFYHASH,
&fnnode_hashmask);
TASK_INIT(&fsnotify_task, 0, process_queue, NULL);
@@ -230,6 +236,28 @@
destroy_dev(fsnotify_dev);
taskqueue_drain(fsnotify_tq, &fsnotify_task);
taskqueue_free(fsnotify_tq);
+ for (i = 0; i <= fnnode_hashmask; i++) {
+ hashhead = &fnnode_inohashtbl[i];
+ while (!LIST_EMPTY(hashhead)) {
+ node = LIST_FIRST(hashhead);
+ mtx_lock(&node->nd_mtx);
+ if (node->nd_vnode != NULL) {
+ VI_LOCK(node->nd_vnode);
+ node->nd_vnode->v_fnnode = NULL;
+ printf("fsnotify unload: deref vp: node %p vp %p\n",
+ node, node->nd_vnode);
+ VI_UNLOCK(node->nd_vnode);
+ node->nd_vnode = NULL;
+ mtx_unlock(&node->nd_mtx);
+ node_drop(node);
+ } else
+ mtx_unlock(&node->nd_mtx);
+ KASSERT(LIST_FIRST(hashhead)->nd_refcnt == 1,
+ ("Invalid node reference count: %d",
+ LIST_FIRST(hashhead)->nd_refcnt));
+ node_drop(LIST_FIRST(hashhead));
+ }
+ }
free(fnnode_inohashtbl, M_FSNOTIFYHASH);
mtx_destroy(&fsnotify_queue_mtx);
mtx_destroy(&fnnode_hashmtx);
@@ -262,17 +290,27 @@
fsnotify_session_dtor(void *data)
{
struct fnsession *ss = data;
+ struct fnwatch *watch;
struct fneventhandle *eh;
+ printf("session_dtor: %p\n", ss);
+ mtx_lock(&ss->ss_mtx);
while (!TAILQ_EMPTY(&ss->ss_queue)) {
eh = TAILQ_FIRST(&ss->ss_queue);
session_drophandle(ss, eh);
}
+ while (!TAILQ_EMPTY(&ss->ss_watchlist)) {
+ watch = TAILQ_FIRST(&ss->ss_watchlist);
+ watch_free(watch);
+ }
+ mtx_unlock(&ss->ss_mtx);
+
cv_destroy(&ss->ss_queuecv);
mtx_destroy(&ss->ss_mtx);
free(ss, M_FSNOTIFY);
+ printf("session free: %p\n", ss);
}
static int
@@ -281,6 +319,7 @@
struct fnsession *ss;
ss = malloc(sizeof(struct fnsession), M_FSNOTIFY, M_WAITOK | M_ZERO);
+ printf("session alloc: %p\n", ss);
mtx_init(&ss->ss_mtx, "fnsession_queue", NULL, MTX_DEF);
cv_init(&ss->ss_queuecv, "fnsession_queuecv");
@@ -289,6 +328,8 @@
devfs_set_cdevpriv(ss, fsnotify_session_dtor);
+ printf("fsnotify_open: session %p\n", ss);
+
return (0);
}
@@ -300,9 +341,11 @@
struct fnsession *ss;
struct fnwatch *watch;
struct fsnotify_event *fe;
- int len, error;
+ int destroy, len, error;
char user_buf[sizeof(struct fsnotify_event) + MAXPATHLEN];
+ printf("fsnotify_read: offset %jd\n", uio->uio_offset);
+
if (uio->uio_resid == 0)
return (0);
@@ -310,8 +353,6 @@
if (error != 0)
return (error);
- printf("fsnotify_read: offset %jd\n", uio->uio_offset);
-
mtx_lock(&ss->ss_mtx);
if (TAILQ_EMPTY(&ss->ss_queue)) {
@@ -336,8 +377,8 @@
fe->fe_fileno = event->ev_node->nd_ino;
fe->fe_cookie = event->ev_cookie;
event_copypath(event, fe->fe_name, &fe->fe_namelen);
- fe->fe_namelen += 1;
len = fe->fe_namelen + sizeof(struct fsnotify_event);
+ destroy = event->ev_mask & FE_DESTROY;
mtx_unlock(&ss->ss_mtx);
@@ -348,6 +389,8 @@
uio->uio_offset = 0;
mtx_lock(&ss->ss_mtx);
session_drophandle(ss, eh);
+ if (destroy != 0)
+ watch_free(watch);
mtx_unlock(&ss->ss_mtx);
}
@@ -391,6 +434,8 @@
vfslocked = VFS_LOCK_GIANT(vp->v_mount);
node = node_lookupex(vp, &ino, LOOKUP_IGNINVAL);
if (node != NULL) {
+ node_watchhold(node);
+ mtx_unlock(&node->nd_mtx);
VFS_UNLOCK_GIANT(vfslocked);
} else {
error = vn_fullpath(td, vp, &path, &pathfree);
@@ -401,8 +446,11 @@
node->nd_path = path;
node->nd_pathlen = strlen(path);
node->nd_pathfree = pathfree;
+ node_watchhold(node);
+ mtx_unlock(&node->nd_mtx);
}
error = session_addwatch(ss, node, add_args->fa_mask, &watch);
+ node_drop(node);
if (error == 0)
add_args->fa_wd = watch->wt_wd;
/* If error != 0 node will be removed by hook_reclaim */
@@ -465,8 +513,14 @@
vp->v_fnnode = NULL;
VI_UNLOCK(vp);
- if (NODE_ISVALID(node))
+ if (NODE_ISVALID(node)) {
+ printf("node reclaim: deref vnode: node %p vp %p\n",
+ node, node->nd_vnode);
+ mtx_lock(&node->nd_mtx);
+ node->nd_vnode = NULL;
+ mtx_unlock(&node->nd_mtx);
node_drop(node);
+ }
}
static __inline int
@@ -489,6 +543,7 @@
int cookie;
cookie = event_nextcookie();
+ printf("hook_generic_remove: %s\n", cnp->cn_nameptr);
node = node_lookup(vp);
if (node != NULL)
@@ -502,6 +557,7 @@
static int
hook_create(struct vop_create_args *ap)
{
+ printf("hook_create: %s\n", ap->a_cnp->cn_nameptr);
hook_generic_create(ap->a_dvp, *ap->a_vpp, ap->a_cnp);
return (0);
}
@@ -509,6 +565,7 @@
static int
hook_mkdir(struct vop_mkdir_args *ap)
{
+ printf("hook_mkdir: %s\n", ap->a_cnp->cn_nameptr);
hook_generic_create(ap->a_dvp, *ap->a_vpp, ap->a_cnp);
return (0);
}
@@ -516,6 +573,7 @@
static int
hook_link(struct vop_link_args *ap)
{
+ printf("hook_link: %s\n", ap->a_cnp->cn_nameptr);
hook_generic_create(ap->a_tdvp, ap->a_vp, ap->a_cnp);
return (0);
}
@@ -523,6 +581,7 @@
static int
hook_symlink(struct vop_symlink_args *ap)
{
+ printf("hook_symlink: %s\n", ap->a_cnp->cn_nameptr);
hook_generic_create(ap->a_dvp, *ap->a_vpp, ap->a_cnp);
return (0);
}
@@ -557,6 +616,7 @@
}
fnode = node_lookupex(ap->a_fvp, NULL, 0);
if (fnode != NULL) {
+ mtx_unlock(&fnode->nd_mtx);
/* TODO */
/* mark path stale */
}
@@ -571,13 +631,6 @@
return (0);
}
-static void
-watch_tryfree(struct fnwatch *watch)
-{
- if (watch->wt_session == NULL && watch->wt_node == NULL)
- free(watch, M_FSNOTIFY);
-}
-
static int
watch_nextwd(void)
{
@@ -595,26 +648,99 @@
return (nwd);
}
+static void
+watch_detachnode(struct fnwatch *watch)
+{
+ struct fnnode *node = watch->wt_node;
+
+ mtx_assert(&node->nd_mtx, MA_OWNED);
+
+ TAILQ_REMOVE(&node->nd_watchlist, watch, wt_nodeentry);
+ node->nd_watchcount--;
+ watch->wt_node = NULL;
+ MPASS(watch->wt_session != NULL);
+}
+
+static void
+watch_free(struct fnwatch *watch)
+{
+ struct fnsession *ss;
+ struct fnnode *node;
+ struct fneventhandle *eh;
+
+ ss = watch->wt_session;
+ node = watch->wt_node;
+ printf("watch_free: watch %p: session %p node %p\n", watch, ss, node);
+ mtx_assert(&ss->ss_mtx, MA_OWNED);
+
+ if (node != NULL) {
+ mtx_lock(&node->nd_mtx);
+ if (watch->wt_node != NULL) {
+ MPASS(watch->wt_node == node);
+ watch_detachnode(watch);
+ node_watchdrop(node);
+ } else
+ mtx_unlock(&node->nd_mtx);
+ }
+
+ TAILQ_REMOVE(&ss->ss_watchlist, watch, wt_sessionentry);
+ watch->wt_session = NULL;
+
+ TAILQ_FOREACH(eh, &ss->ss_queue, eh_queueentry) {
+ MPASS(eh->eh_watch != watch);
+ }
+
+ MPASS(watch->wt_session == NULL && watch->wt_node == NULL);
+ printf("watch_free: free %p\n", watch);
+ free(watch, M_FSNOTIFY);
+}
+
static __inline void
node_hold(struct fnnode *node)
{
refcount_acquire(&node->nd_refcnt);
}
+static __inline void
+node_watchhold(struct fnnode *node)
+{
+ mtx_assert(&node->nd_mtx, MA_OWNED);
+ node_hold(node);
+ if (TAILQ_EMPTY(&node->nd_watchlist))
+ node_hold(node);
+}
+
+static __inline void
+node_watchdrop(struct fnnode *node)
+{
+ mtx_assert(&node->nd_mtx, MA_OWNED);
+ if (TAILQ_EMPTY(&node->nd_watchlist)) {
+ MPASS(node->nd_watchcount == 0);
+ node->nd_supermask = 0;
+ mtx_unlock(&node->nd_mtx);
+ node_drop(node);
+ } else
+ mtx_unlock(&node->nd_mtx);
+}
+
static void
node_drop(struct fnnode *node)
{
+ mtx_assert(&node->nd_mtx, MA_NOTOWNED);
if (refcount_release(&node->nd_refcnt) != 0) {
- KASSERT(node->nd_watchcount == 0 && node->nd_supermask == 0 &&
- TAILQ_EMPTY(&node->nd_watchlist), ("Invalid reference count"));
+ MPASS(node->nd_vnode == NULL);
+ KASSERT(node->nd_watchcount == 0 &&
+ TAILQ_EMPTY(&node->nd_watchlist),
+ ("Invalid watch count: %d", node->nd_watchcount));
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->nd_pathfree, M_TEMP);
free(node, M_FSNOTIFY);
+ printf("node free: %p\n", node);
}
}
@@ -636,14 +762,23 @@
MPASS(ino != 0);
node = malloc(sizeof(struct fnnode), M_FSNOTIFY, M_WAITOK | M_ZERO);
+ printf("node alloc: node %p vp %p\n", node, vp);
- refcount_init(&node->nd_refcnt, 1);
+ refcount_init(&node->nd_refcnt, 2);
mtx_init(&node->nd_mtx, "fsnotify_node", NULL, MTX_DEF);
TAILQ_INIT(&node->nd_watchlist);
node->nd_ino = ino;
+ mtx_lock(&node->nd_mtx);
+
+ mtx_lock(&fnnode_hashmtx);
+ LIST_INSERT_HEAD(node_inohashhead(vp->v_mount, ino),
+ node, nd_hashentry);
+ mtx_unlock(&fnnode_hashmtx);
+
VI_LOCK(vp);
+ node->nd_vnode = vp;
vp->v_fnnode = node;
VI_UNLOCK(vp);
@@ -651,19 +786,21 @@
}
static void
-node_detachwatches(struct fnnode *node)
+node_detachallwatches(struct fnnode *node)
{
- struct fnwatch *watch;
+ struct fnwatch *watch = NULL;
mtx_assert(&node->nd_mtx, MA_OWNED);
- node->nd_watchcount = 0;
- node->nd_supermask = 0;
while (!TAILQ_EMPTY(&node->nd_watchlist)) {
watch = TAILQ_FIRST(&node->nd_watchlist);
- TAILQ_REMOVE(&node->nd_watchlist, watch, wt_nodeentry);
- watch->wt_node = NULL;
- watch_tryfree(watch);
+ watch_detachnode(watch);
}
+ node->nd_supermask = 0;
+ MPASS(node->nd_watchcount == 0);
+ if (watch != NULL)
+ node_watchdrop(node);
+ else
+ mtx_unlock(&node->nd_mtx);
}
static int
@@ -696,7 +833,7 @@
{
struct fnnode *node, *rv;
ino_t ino;
- int error, watchcount;
+ int error;
rv = NULL;
VI_LOCK(vp);
@@ -708,10 +845,11 @@
goto done;
} else if (node != NULL) {
mtx_lock(&node->nd_mtx);
- watchcount = node->nd_watchcount;
- mtx_unlock(&node->nd_mtx);
- if (watchcount == 0)
+ MPASS(node->nd_vnode == vp);
+ if (node->nd_watchcount == 0) {
+ mtx_unlock(&node->nd_mtx);
node = NULL;
+ }
goto done;
}
@@ -729,18 +867,21 @@
node->nd_mount != vp->v_mount)
continue;
mtx_lock(&node->nd_mtx);
+ mtx_unlock(&fnnode_hashmtx);
VI_LOCK(vp);
if (!NODE_ISVALID(vp->v_fnnode)) {
+ MPASS(node->nd_vnode == NULL);
node_hold(node);
+ printf("node lookup: ref vnode: node %p vp %p\n", node, vp);
vp->v_fnnode = node;
+ node->nd_vnode = vp;
} else
- MPASS(vp->v_fnnode == node);
+ MPASS(vp->v_fnnode == node && vp == node->nd_vnode);
VI_UNLOCK(vp);
- watchcount = node->nd_watchcount;
- mtx_unlock(&node->nd_mtx);
- if (watchcount == 0)
+ if (node->nd_watchcount == 0) {
+ mtx_unlock(&node->nd_mtx);
node = NULL;
- mtx_unlock(&fnnode_hashmtx);
+ }
goto done;
}
mtx_unlock(&fnnode_hashmtx);
@@ -751,6 +892,8 @@
VI_UNLOCK(vp);
done:
+ if (node != NULL)
+ mtx_assert(&node->nd_mtx, MA_OWNED);
return (node);
}
@@ -776,6 +919,8 @@
* thread
*/
vp = node->nd_vnode;
+ printf("node_updatepath: node %p vp %p %s\n",
+ node, vp, node->nd_path);
if ((vp->v_iflag & VI_DOOMED) != 0)
return (ENOENT);
@@ -845,6 +990,7 @@
event->ev_pathpos = MAXPATHLEN - 1 - namelen;
memcpy(event->ev_pathfree + event->ev_pathpos, name, namelen);
event->ev_pathfree[MAXPATHLEN - 1] = '\0';
+ printf("event alloc: %p\n", event);
return (event);
}
@@ -855,6 +1001,7 @@
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
@@ -866,8 +1013,9 @@
static __inline void
event_copypath(struct fnevent *event, char *path, int *pathlen)
{
- *pathlen = event_pathlen(event);
- memcpy(path, event->ev_pathfree + *pathlen, *pathlen);
+ /* Count last zero byte */
+ *pathlen = event_pathlen(event) + 1;
+ memcpy(path, event->ev_pathfree + event->ev_pathpos, *pathlen);
}
static void
@@ -921,12 +1069,14 @@
{
struct fnwatch *watch;
+ printf("session_addwatch: %s: session=%p\n", node->nd_path, ss);
+
watch = malloc(sizeof(struct fnwatch), M_FSNOTIFY, M_WAITOK | M_ZERO);
+ printf("watch alloc: %p\n", watch);
watch->wt_wd = watch_nextwd();
watch->wt_mask = mask;
watch->wt_session = ss;
- node_hold(node);
watch->wt_node = node;
mtx_lock(&ss->ss_mtx);
@@ -948,15 +1098,19 @@
static int
session_rmwatch(struct fnsession *ss, int wd)
{
- struct fnwatch *watch, *tmp;
+ struct fnwatch *watch;
+ struct fneventhandle *eh, *ehtmp;
mtx_lock(&ss->ss_mtx);
- TAILQ_FOREACH_SAFE(watch, &ss->ss_watchlist, wt_sessionentry, tmp) {
+ TAILQ_FOREACH(watch, &ss->ss_watchlist, wt_sessionentry) {
if (watch->wt_wd != wd)
continue;
- TAILQ_REMOVE(&ss->ss_watchlist, watch, wt_sessionentry);
- watch->wt_session = NULL;
- watch_tryfree(watch);
+ TAILQ_FOREACH_SAFE(eh, &ss->ss_queue, eh_queueentry, ehtmp) {
+ if (eh->eh_watch != watch)
+ continue;
+ session_drophandle(ss, eh);
+ }
+ watch_free(watch);
break;
}
mtx_unlock(&ss->ss_mtx);
@@ -981,15 +1135,12 @@
ss = eh->eh_watch->wt_session;
- if (ss == NULL)
+ if (ss == NULL) {
+ eventhandle_drop(eh);
return;
+ }
mtx_lock(&ss->ss_mtx);
- if (eh->eh_watch->wt_session != NULL) {
- mtx_unlock(&ss->ss_mtx);
- eventhandle_drop(eh);
- return;
- }
TAILQ_INSERT_TAIL(&ss->ss_queue, eh, eh_queueentry);
ss->ss_queuesize++;
mtx_unlock(&ss->ss_mtx);
@@ -1004,7 +1155,6 @@
struct fnwatch *watch;
struct fnevent *event;
struct fneventhandle *eh;
- struct vnode *vp;
int i, handle_count;
while (1) {
@@ -1028,6 +1178,8 @@
event->ev_handlemaxsize);
break;
}
+ printf("process_queue: handle event %p in session %p\n",
+ event, watch->wt_session);
eh = &event->ev_handlebuf[event->ev_handlecount++];
eh->eh_event = event;
eh->eh_watch = watch;
@@ -1039,17 +1191,18 @@
event_free(event);
continue;
}
- vp = node->nd_vnode;
- if (vp != NULL)
+ /* FIXME */
+ if (0 && node->nd_vnode != NULL)
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 & FE_DESTROY)
- node_detachwatches(node);
-
- event_prependpath(event, node);
- mtx_unlock(&node->nd_mtx);
+ node_detachallwatches(node);
+ else
+ mtx_unlock(&node->nd_mtx);
for (i = 0; i < handle_count; i++)
session_enqueue(&event->ev_handlebuf[i]);
@@ -1060,12 +1213,12 @@
enqueue_direvent(struct fnnode *dirnode, struct componentname *cnp, int cookie, int mask)
{
struct fnevent *event;
- int supermask, watch_count;
+ int supermask, watchcount;
printf("enqueue_direvent: %s %x\n", cnp->cn_nameptr, mask);
mtx_assert(&dirnode->nd_mtx, MA_OWNED);
- watch_count = dirnode->nd_watchcount;
+ watchcount = dirnode->nd_watchcount;
supermask = dirnode->nd_supermask & mask;
node_hold(dirnode);
mtx_unlock(&dirnode->nd_mtx);
@@ -1075,10 +1228,10 @@
return;
}
- KASSERT(watch_count > 0, ("No watchers found"));
+ KASSERT(watchcount > 0, ("No watchers found"));
event = event_alloc(dirnode, cnp->cn_nameptr, cnp->cn_namelen,
- watch_count + 1, mask, cookie);
+ watchcount + 1, mask, cookie);
mtx_lock(&fsnotify_queue_mtx);
TAILQ_INSERT_TAIL(&fsnotify_queue, event, ev_queueentry);
More information about the p4-projects
mailing list