Module Name: src
Committed By: manu
Date: Fri Nov 16 02:39:02 UTC 2018
Modified Files:
src/lib/libperfuse: debug.c ops.c perfuse.c perfuse_priv.h
Log Message:
Use reclaim2 to fix reclaim/lookup race conditions
The PUFFS reclaim operation had a race condition with lookups: we could
be asked to lookup a node, then to reclaim it before lookup completion.
At lookup completion, we would then create a leaked node.
Enter the PUFFS reclaim2 operation, which features a nlookup argument.
That let us count how many lookups are pending and avoid the above
described scenario. It also makes the codes simplier.
To generate a diff of this commit:
cvs rdiff -u -r1.12 -r1.13 src/lib/libperfuse/debug.c
cvs rdiff -u -r1.84 -r1.85 src/lib/libperfuse/ops.c
cvs rdiff -u -r1.40 -r1.41 src/lib/libperfuse/perfuse.c
cvs rdiff -u -r1.36 -r1.37 src/lib/libperfuse/perfuse_priv.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/lib/libperfuse/debug.c
diff -u src/lib/libperfuse/debug.c:1.12 src/lib/libperfuse/debug.c:1.13
--- src/lib/libperfuse/debug.c:1.12 Sat Jul 21 05:49:42 2012
+++ src/lib/libperfuse/debug.c Fri Nov 16 02:39:02 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: debug.c,v 1.12 2012/07/21 05:49:42 manu Exp $ */
+/* $NetBSD: debug.c,v 1.13 2018/11/16 02:39:02 manu Exp $ */
/*-
* Copyright (c) 2010 Emmanuel Dreyfus. All rights reserved.
@@ -270,7 +270,6 @@ perfuse_trace_dump(struct puffs_usermoun
fprintf(fp, "\n\nGlobal statistics\n");
fprintf(fp, "Nodes: %d\n", ps->ps_nodecount);
fprintf(fp, "Exchanges: %d\n", ps->ps_xchgcount);
- fprintf(fp, "Nodes possibly leaked: %d\n", ps->ps_nodeleakcount);
(void)fflush(fp);
return;
Index: src/lib/libperfuse/ops.c
diff -u src/lib/libperfuse/ops.c:1.84 src/lib/libperfuse/ops.c:1.85
--- src/lib/libperfuse/ops.c:1.84 Wed Jun 3 14:07:05 2015
+++ src/lib/libperfuse/ops.c Fri Nov 16 02:39:02 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: ops.c,v 1.84 2015/06/03 14:07:05 manu Exp $ */
+/* $NetBSD: ops.c,v 1.85 2018/11/16 02:39:02 manu Exp $ */
/*-
* Copyright (c) 2010-2011 Emmanuel Dreyfus. All rights reserved.
@@ -500,11 +500,6 @@ node_lookup_common(struct puffs_usermoun
puffs_newinfo_setrdev(pni, pn->pn_va.va_rdev);
}
- if (PERFUSE_NODE_DATA(pn)->pnd_flags & PND_NODELEAK) {
- PERFUSE_NODE_DATA(pn)->pnd_flags &= ~PND_NODELEAK;
- ps->ps_nodeleakcount--;
- }
-
ps->ps_destroy_msg(pm);
return 0;
@@ -672,7 +667,7 @@ fuse_to_dirent(struct puffs_usermount *p
"failed: %d", name, error);
} else {
fd->ino = pn->pn_va.va_fileid;
- (void)perfuse_node_reclaim(pu, pn);
+ (void)perfuse_node_reclaim2(pu, pn, 1);
}
}
}
@@ -1135,7 +1130,7 @@ perfuse_node_lookup(struct puffs_usermou
case NAMEI_RENAME:
error = sticky_access(opc, pn, pcn->pcn_cred);
if (error != 0) {
- (void)perfuse_node_reclaim(pu, pn);
+ (void)perfuse_node_reclaim2(pu, pn, 1);
goto out;
}
break;
@@ -1182,7 +1177,7 @@ perfuse_node_create(struct puffs_usermou
error = node_lookup_common(pu, opc, NULL, pcn->pcn_name,
pcn->pcn_cred, &pn);
if (error == 0) {
- (void)perfuse_node_reclaim(pu, pn);
+ (void)perfuse_node_reclaim2(pu, pn, 1);
error = EEXIST;
goto out;
}
@@ -2682,17 +2677,22 @@ out:
}
int
-perfuse_node_reclaim(struct puffs_usermount *pu, puffs_cookie_t opc)
+perfuse_node_reclaim2(struct puffs_usermount *pu,
+ puffs_cookie_t opc, int nlookup)
{
struct perfuse_state *ps;
perfuse_msg_t *pm;
struct perfuse_node_data *pnd;
struct fuse_forget_in *ffi;
- int nlookup;
- struct timespec now;
- if (opc == 0)
+#ifdef PERFUSE_DEBUG
+ if (perfuse_diagflags & PDF_RECLAIM)
+ DPRINTF("%s called with opc = %p, nlookup = %d\n",
+ __func__, (void *)opc, nlookup);
+#endif
+ if (opc == 0 || nlookup == 0) {
return 0;
+ }
ps = puffs_getspecific(pu);
pnd = PERFUSE_NODE_DATA(opc);
@@ -2703,43 +2703,23 @@ perfuse_node_reclaim(struct puffs_usermo
if (pnd->pnd_nodeid == FUSE_ROOT_ID)
return 0;
+#ifdef PERFUSE_DEBUG
+ if (perfuse_diagflags & PDF_RECLAIM)
+ DPRINTF("%s (nodeid %"PRId64") reclaimed, nlookup = %d/%d\n",
+ perfuse_node_path(ps, opc), pnd->pnd_nodeid,
+ nlookup, pnd->pnd_puffs_nlookup);
+#endif
/*
- * There is a race condition between reclaim and lookup.
- * When looking up an already known node, the kernel cannot
- * hold a reference on the result until it gets the PUFFS
- * reply. It mayy therefore reclaim the node after the
- * userland looked it up, and before it gets the reply.
- * On rely, the kernel re-creates the node, but at that
- * time the node has been reclaimed in userland.
- *
- * In order to avoid this, we refuse reclaiming nodes that
- * are too young since the last lookup - and that we do
- * not have removed on our own, of course.
- */
- if (clock_gettime(CLOCK_REALTIME, &now) != 0)
- DERR(EX_OSERR, "clock_gettime failed");
-
- if (timespeccmp(&pnd->pnd_cn_expire, &now, >) &&
- !(pnd->pnd_flags & PND_REMOVED)) {
- if (!(pnd->pnd_flags & PND_NODELEAK)) {
- ps->ps_nodeleakcount++;
- pnd->pnd_flags |= PND_NODELEAK;
- }
- DWARNX("possible leaked node:: opc = %p \"%s\"",
- opc, pnd->pnd_name);
+ * The kernel tells us how many lookups it made, which allows
+ * us to detect that we have an uncompleted lookup and that the
+ * node should not dispear.
+ */
+ pnd->pnd_puffs_nlookup -= nlookup;
+ if (pnd->pnd_puffs_nlookup > 0)
return 0;
- }
node_ref(opc);
pnd->pnd_flags |= PND_RECLAIMED;
- pnd->pnd_puffs_nlookup--;
- nlookup = pnd->pnd_puffs_nlookup;
-
-#ifdef PERFUSE_DEBUG
- if (perfuse_diagflags & PDF_RECLAIM)
- DPRINTF("%s (nodeid %"PRId64") reclaimed\n",
- perfuse_node_path(ps, opc), pnd->pnd_nodeid);
-#endif
#ifdef PERFUSE_DEBUG
if (perfuse_diagflags & PDF_RECLAIM)
@@ -2751,7 +2731,7 @@ perfuse_node_reclaim(struct puffs_usermo
pnd->pnd_flags & PND_OPEN ? "open " : "not open",
pnd->pnd_flags & PND_RFH ? "r" : "",
pnd->pnd_flags & PND_WFH ? "w" : "",
- pnd->pnd_flags & PND_BUSY ? "" : " none",
+ pnd->pnd_flags & PND_BUSY ? " busy" : "",
pnd->pnd_flags & PND_INREADDIR ? " readdir" : "",
pnd->pnd_flags & PND_INWRITE ? " write" : "",
pnd->pnd_flags & PND_INOPEN ? " open" : "");
@@ -2769,17 +2749,6 @@ perfuse_node_reclaim(struct puffs_usermo
while (pnd->pnd_ref > 1)
requeue_request(pu, opc, PCQ_REF);
- /*
- * reclaim cancel?
- */
- if (pnd->pnd_puffs_nlookup > nlookup) {
- pnd->pnd_flags &= ~PND_RECLAIMED;
- perfuse_node_cache(ps, opc);
- node_rele(opc);
- return 0;
- }
-
-
#ifdef PERFUSE_DEBUG
if ((pnd->pnd_flags & PND_OPEN) ||
!TAILQ_EMPTY(&pnd->pnd_pcq))
@@ -2818,6 +2787,16 @@ perfuse_node_reclaim(struct puffs_usermo
return 0;
}
+int
+perfuse_node_reclaim(struct puffs_usermount *pu, puffs_cookie_t opc)
+{
+#ifdef PERFUSE_DEBUG
+ if (perfuse_diagflags & PDF_RECLAIM)
+ DPRINTF("perfuse_node_reclaim called\n");
+#endif
+ return perfuse_node_reclaim2(pu, opc, 1);
+}
+
int
perfuse_node_inactive(struct puffs_usermount *pu, puffs_cookie_t opc)
{
Index: src/lib/libperfuse/perfuse.c
diff -u src/lib/libperfuse/perfuse.c:1.40 src/lib/libperfuse/perfuse.c:1.41
--- src/lib/libperfuse/perfuse.c:1.40 Wed Oct 19 01:30:35 2016
+++ src/lib/libperfuse/perfuse.c Fri Nov 16 02:39:02 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: perfuse.c,v 1.40 2016/10/19 01:30:35 christos Exp $ */
+/* $NetBSD: perfuse.c,v 1.41 2018/11/16 02:39:02 manu Exp $ */
/*-
* Copyright (c) 2010-2011 Emmanuel Dreyfus. All rights reserved.
@@ -512,6 +512,7 @@ perfuse_init(struct perfuse_callbacks *p
PUFFSOP_SET(pops, perfuse, node, readdir);
PUFFSOP_SET(pops, perfuse, node, readlink);
PUFFSOP_SET(pops, perfuse, node, reclaim);
+ PUFFSOP_SET(pops, perfuse, node, reclaim2);
PUFFSOP_SET(pops, perfuse, node, inactive);
PUFFSOP_SET(pops, perfuse, node, print);
PUFFSOP_SET(pops, perfuse, node, pathconf);
Index: src/lib/libperfuse/perfuse_priv.h
diff -u src/lib/libperfuse/perfuse_priv.h:1.36 src/lib/libperfuse/perfuse_priv.h:1.37
--- src/lib/libperfuse/perfuse_priv.h:1.36 Fri Oct 31 15:12:15 2014
+++ src/lib/libperfuse/perfuse_priv.h Fri Nov 16 02:39:02 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: perfuse_priv.h,v 1.36 2014/10/31 15:12:15 manu Exp $ */
+/* $NetBSD: perfuse_priv.h,v 1.37 2018/11/16 02:39:02 manu Exp $ */
/*-
* Copyright (c) 2010-2011 Emmanuel Dreyfus. All rights reserved.
@@ -93,7 +93,6 @@ struct perfuse_state {
struct perfuse_node_hashlist *ps_nidhash;
int ps_nnidhash;
int ps_nodecount;
- int ps_nodeleakcount;
int ps_xchgcount;
};
@@ -145,7 +144,6 @@ struct perfuse_node_data {
#define PND_REMOVED 0x020 /* Node was removed */
#define PND_INWRITE 0x040 /* write in progress */
#define PND_INOPEN 0x100 /* open in progress */
-#define PND_NODELEAK 0x200 /* node reclaim ignored */
#define PND_INVALID 0x400 /* node freed, usage is a bug */
#define PND_INRESIZE 0x800 /* resize in progress */
@@ -247,6 +245,7 @@ int perfuse_node_readdir(struct puffs_us
int perfuse_node_readlink(struct puffs_usermount *,
puffs_cookie_t, const struct puffs_cred *, char *, size_t *);
int perfuse_node_reclaim(struct puffs_usermount *, puffs_cookie_t);
+int perfuse_node_reclaim2(struct puffs_usermount *, puffs_cookie_t, int);
int perfuse_node_inactive(struct puffs_usermount *, puffs_cookie_t);
int perfuse_node_print(struct puffs_usermount *, puffs_cookie_t);
int perfuse_node_pathconf(struct puffs_usermount *,
