Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/netbsd-8]: src/lib/libperfuse Pull up following revision(s) (requested b...
details: https://anonhg.NetBSD.org/src/rev/8c00513bea59
branches: netbsd-8
changeset: 448831:8c00513bea59
user: martin <martin%NetBSD.org@localhost>
date: Sun Feb 10 13:40:41 2019 +0000
description:
Pull up following revision(s) (requested by manu in ticket #1186):
lib/libperfuse/perfuse.c: revision 1.41
lib/libperfuse/perfuse_priv.h: revision 1.37
lib/libperfuse/ops.c: revision 1.85
lib/libperfuse/ops.c: revision 1.86
lib/libperfuse/debug.c: revision 1.13
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.
-
Fix directory filehandle usage with libufse. Fix lookup count
libfuse does not use filehandle the same way for directories and other
objects. As a result, filehandles obtained by OPENDIR should not be
sent on non-directory related operations like READ/WRITE/GETATTR...
While there, fix the lookup count sent to the FORGET operation, which
led to leaked nodes.
diffstat:
lib/libperfuse/debug.c | 3 +-
lib/libperfuse/ops.c | 151 ++++++++++++++++++++---------------------
lib/libperfuse/perfuse.c | 3 +-
lib/libperfuse/perfuse_priv.h | 5 +-
4 files changed, 80 insertions(+), 82 deletions(-)
diffs (truncated from 477 to 300 lines):
diff -r ec9db8714e76 -r 8c00513bea59 lib/libperfuse/debug.c
--- a/lib/libperfuse/debug.c Sat Feb 09 14:43:40 2019 +0000
+++ b/lib/libperfuse/debug.c Sun Feb 10 13:40:41 2019 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: debug.c,v 1.12 2012/07/21 05:49:42 manu Exp $ */
+/* $NetBSD: debug.c,v 1.12.24.1 2019/02/10 13:40:41 martin 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 ec9db8714e76 -r 8c00513bea59 lib/libperfuse/ops.c
--- a/lib/libperfuse/ops.c Sat Feb 09 14:43:40 2019 +0000
+++ b/lib/libperfuse/ops.c Sun Feb 10 13:40:41 2019 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ops.c,v 1.84 2015/06/03 14:07:05 manu Exp $ */
+/* $NetBSD: ops.c,v 1.84.8.1 2019/02/10 13:40:41 martin Exp $ */
/*-
* Copyright (c) 2010-2011 Emmanuel Dreyfus. All rights reserved.
@@ -105,6 +105,9 @@
#define IFTOVT(mode) (iftovt_tab[((mode) & S_IFMT) >> 12])
#define VTTOIF(indx) (vttoif_tab[(int)(indx)])
+#define PN_ISDIR(opc) \
+ (puffs_pn_getvap((struct puffs_node *)opc)->va_type == VDIR)
+
#if 0
static void
print_node(const char *func, puffs_cookie_t opc)
@@ -141,7 +144,7 @@
pn = (struct puffs_node *)opc;
pnd = PERFUSE_NODE_DATA(pn);
- if (puffs_pn_getvap(pn)->va_type == VDIR) {
+ if (PN_ISDIR(opc)) {
op = FUSE_RELEASEDIR;
mode = FREAD;
} else {
@@ -479,13 +482,14 @@
fuse_attr_to_vap(ps, &pn->pn_va, &feo->attr);
pn->pn_va.va_gen = (u_long)(feo->generation);
PERFUSE_NODE_DATA(pn)->pnd_fuse_nlookup++;
+ PERFUSE_NODE_DATA(pn)->pnd_puffs_nlookup++;
*pnp = pn;
#ifdef PERFUSE_DEBUG
if (perfuse_diagflags & PDF_FILENAME)
DPRINTF("%s: opc = %p, looked up opc = %p, "
- "nodeid = 0x%"PRIx64" file = \"%s\"\n", __func__,
+ "nodeid = 0x%"PRIx64" file = \"%s\"\n", __func__,
(void *)opc, pn, feo->nodeid, path);
#endif
@@ -500,11 +504,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;
@@ -538,6 +537,7 @@
pn = perfuse_new_pn(pu, pcn->pcn_name, opc);
PERFUSE_NODE_DATA(pn)->pnd_nodeid = feo->nodeid;
+ PERFUSE_NODE_DATA(pn)->pnd_fuse_nlookup++;
PERFUSE_NODE_DATA(pn)->pnd_puffs_nlookup++;
perfuse_node_cache(ps, pn);
@@ -672,7 +672,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 +1135,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;
@@ -1143,6 +1143,7 @@
break;
}
+ PERFUSE_NODE_DATA(pn)->pnd_fuse_nlookup++;
PERFUSE_NODE_DATA(pn)->pnd_puffs_nlookup++;
error = 0;
@@ -1182,7 +1183,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;
}
@@ -1252,6 +1253,7 @@
pn = perfuse_new_pn(pu, name, opc);
perfuse_new_fh((puffs_cookie_t)pn, foo->fh, FWRITE);
PERFUSE_NODE_DATA(pn)->pnd_nodeid = feo->nodeid;
+ PERFUSE_NODE_DATA(pn)->pnd_fuse_nlookup++;
PERFUSE_NODE_DATA(pn)->pnd_puffs_nlookup++;
perfuse_node_cache(ps, pn);
@@ -1360,11 +1362,9 @@
int op;
struct fuse_open_in *foi;
struct fuse_open_out *foo;
- struct puffs_node *pn;
int error;
ps = puffs_getspecific(pu);
- pn = (struct puffs_node *)opc;
pnd = PERFUSE_NODE_DATA(opc);
error = 0;
@@ -1373,7 +1373,7 @@
node_ref(opc);
- if (puffs_pn_getvap(pn)->va_type == VDIR)
+ if (PN_ISDIR(opc))
op = FUSE_OPENDIR;
else
op = FUSE_OPEN;
@@ -1597,9 +1597,9 @@
fgi = GET_INPAYLOAD(ps, pm, fuse_getattr_in);
fgi->getattr_flags = 0;
fgi->dummy = 0;
- fgi->fh = 0;
-
- if (PERFUSE_NODE_DATA(opc)->pnd_flags & PND_OPEN) {
+ fgi->fh = FUSE_UNKNOWN_FH;
+
+ if (!PN_ISDIR(opc) && PERFUSE_NODE_DATA(opc)->pnd_flags & PND_OPEN) {
fgi->fh = perfuse_get_fh(opc, FREAD);
fgi->getattr_flags |= FUSE_GETATTR_FH;
}
@@ -1733,7 +1733,7 @@
node_ref(opc);
- if (pnd->pnd_flags & PND_WFH)
+ if (!PN_ISDIR(opc) && pnd->pnd_flags & PND_WFH)
fh = perfuse_get_fh(opc, FWRITE);
else
fh = FUSE_UNKNOWN_FH;
@@ -1959,7 +1959,7 @@
*/
pm = ps->ps_new_msg(pu, opc, FUSE_POLL, sizeof(*fpi), NULL);
fpi = GET_INPAYLOAD(ps, pm, fuse_poll_in);
- fpi->fh = perfuse_get_fh(opc, FREAD);
+ fpi->fh = PN_ISDIR(opc) ? FUSE_UNKNOWN_FH : perfuse_get_fh(opc, FREAD);
fpi->kh = 0;
fpi->flags = 0;
@@ -2015,7 +2015,7 @@
node_ref(opc);
- if (puffs_pn_getvap((struct puffs_node *)opc)->va_type == VDIR)
+ if (PN_ISDIR(opc))
op = FUSE_FSYNCDIR;
else /* VREG but also other types such as VLNK */
op = FUSE_FSYNC;
@@ -2518,7 +2518,7 @@
goto out;
}
- fh = perfuse_get_fh(opc, FREAD);
+ fh = perfuse_get_fh(opc, FREAD);
#ifdef PERFUSE_DEBUG
if (perfuse_diagflags & PDF_FH)
@@ -2682,17 +2682,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 +2708,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 +2736,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 +2754,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;
- }
-
-
Home |
Main Index |
Thread Index |
Old Index