Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/lib/libperfuse Use reclaim2 to fix reclaim/lookup race condi...
details: https://anonhg.NetBSD.org/src/rev/792d13c64872
branches: trunk
changeset: 445882:792d13c64872
user: manu <manu%NetBSD.org@localhost>
date: Fri Nov 16 02:39:02 2018 +0000
description:
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.
diffstat:
lib/libperfuse/debug.c | 3 +-
lib/libperfuse/ops.c | 91 ++++++++++++++++--------------------------
lib/libperfuse/perfuse.c | 3 +-
lib/libperfuse/perfuse_priv.h | 5 +-
4 files changed, 40 insertions(+), 62 deletions(-)
diffs (241 lines):
diff -r 0faa47b8014a -r 792d13c64872 lib/libperfuse/debug.c
--- a/lib/libperfuse/debug.c Fri Nov 16 00:34:50 2018 +0000
+++ b/lib/libperfuse/debug.c Fri Nov 16 02:39:02 2018 +0000
@@ -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 @@
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;
diff -r 0faa47b8014a -r 792d13c64872 lib/libperfuse/ops.c
--- a/lib/libperfuse/ops.c Fri Nov 16 00:34:50 2018 +0000
+++ b/lib/libperfuse/ops.c Fri Nov 16 02:39:02 2018 +0000
@@ -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 @@
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 @@
"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 @@
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 @@
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 @@
}
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 @@
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.
+ * 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.
*/
- 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);
+ 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 @@
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 @@
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 @@
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)
{
diff -r 0faa47b8014a -r 792d13c64872 lib/libperfuse/perfuse.c
--- a/lib/libperfuse/perfuse.c Fri Nov 16 00:34:50 2018 +0000
+++ b/lib/libperfuse/perfuse.c Fri Nov 16 02:39:02 2018 +0000
@@ -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 @@
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);
diff -r 0faa47b8014a -r 792d13c64872 lib/libperfuse/perfuse_priv.h
--- a/lib/libperfuse/perfuse_priv.h Fri Nov 16 00:34:50 2018 +0000
+++ b/lib/libperfuse/perfuse_priv.h Fri Nov 16 02:39:02 2018 +0000
@@ -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_node_hashlist *ps_nidhash;
int ps_nnidhash;
int ps_nodecount;
- int ps_nodeleakcount;
int ps_xchgcount;
};
@@ -145,7 +144,6 @@
#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_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 *,
Home |
Main Index |
Thread Index |
Old Index