Module Name: src
Committed By: martin
Date: Sun Feb 10 13:40:41 UTC 2019
Modified Files:
src/lib/libperfuse [netbsd-8]: debug.c ops.c perfuse.c perfuse_priv.h
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.12 -r1.12.24.1 src/lib/libperfuse/debug.c
cvs rdiff -u -r1.84 -r1.84.8.1 src/lib/libperfuse/ops.c
cvs rdiff -u -r1.40 -r1.40.6.1 src/lib/libperfuse/perfuse.c
cvs rdiff -u -r1.36 -r1.36.8.1 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.12.24.1
--- src/lib/libperfuse/debug.c:1.12 Sat Jul 21 05:49:42 2012
+++ src/lib/libperfuse/debug.c Sun Feb 10 13:40:41 2019
@@ -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 @@ 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.84.8.1
--- src/lib/libperfuse/ops.c:1.84 Wed Jun 3 14:07:05 2015
+++ src/lib/libperfuse/ops.c Sun Feb 10 13:40:41 2019
@@ -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 @@ const int vttoif_tab[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 @@ perfuse_node_close_common(struct puffs_u
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 @@ node_lookup_common(struct puffs_usermoun
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 @@ 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;
@@ -538,6 +537,7 @@ node_mk_common(struct puffs_usermount *p
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 @@ 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 +1135,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;
@@ -1143,6 +1143,7 @@ perfuse_node_lookup(struct puffs_usermou
break;
}
+ PERFUSE_NODE_DATA(pn)->pnd_fuse_nlookup++;
PERFUSE_NODE_DATA(pn)->pnd_puffs_nlookup++;
error = 0;
@@ -1182,7 +1183,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;
}
@@ -1252,6 +1253,7 @@ perfuse_node_create(struct puffs_usermou
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 @@ perfuse_node_open2(struct puffs_usermoun
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 @@ perfuse_node_open2(struct puffs_usermoun
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 @@ perfuse_node_getattr_ttl(struct puffs_us
fgi = GET_INPAYLOAD(ps, pm, fuse_getattr_in);
fgi->getattr_flags = 0;
fgi->dummy = 0;
- fgi->fh = 0;
+ fgi->fh = FUSE_UNKNOWN_FH;
- if (PERFUSE_NODE_DATA(opc)->pnd_flags & PND_OPEN) {
+ 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 @@ perfuse_node_setattr_ttl(struct puffs_us
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 @@ perfuse_node_poll(struct puffs_usermount
*/
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 @@ perfuse_node_fsync(struct puffs_usermoun
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 @@ perfuse_node_readdir(struct puffs_usermo
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 @@ 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 +2708,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 +2736,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 +2754,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 +2792,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)
{
@@ -2992,15 +2976,24 @@ perfuse_node_advlock(struct puffs_usermo
* expect one. E.g.: if we provide none, GlusterFS logs an error
* "0-glusterfs-fuse: xl is NULL"
*
+ * There is one exception with directories where filehandle
+ * is not included, because libfuse uses different filehandle
+ * in opendir/releasedir/readdir/fsyncdir compared to other
+ * operations. Who locks a directory anyway?
+ *
* We need the read file handle if the file is open read only,
* in order to support shared locks on read-only files.
* NB: The kernel always sends advlock for read-only
* files at exit time when the process used lock, see
* sys_exit -> exit1 -> fd_free -> fd_close -> VOP_ADVLOCK
*/
- if ((fh = perfuse_get_fh(opc, FREAD)) == FUSE_UNKNOWN_FH) {
- error = EBADF;
- goto out;
+ if (!PN_ISDIR(opc)) {
+ if ((fh = perfuse_get_fh(opc, FREAD)) == FUSE_UNKNOWN_FH) {
+ error = EBADF;
+ goto out;
+ }
+ } else {
+ fh = FUSE_UNKNOWN_FH;
}
ps = puffs_getspecific(pu);
@@ -3097,6 +3090,7 @@ perfuse_node_read(struct puffs_usermount
struct perfuse_node_data *pnd;
const struct vattr *vap;
perfuse_msg_t *pm;
+ uint64_t fh;
struct fuse_read_in *fri;
struct fuse_out_header *foh;
size_t readen;
@@ -3113,6 +3107,8 @@ perfuse_node_read(struct puffs_usermount
if (vap->va_type == VDIR)
return EISDIR;
+ fh = perfuse_get_fh(opc, FREAD); /* Cannot be VDIR */
+
do {
size_t max_read;
@@ -3123,7 +3119,7 @@ perfuse_node_read(struct puffs_usermount
*/
pm = ps->ps_new_msg(pu, opc, FUSE_READ, sizeof(*fri), pcr);
fri = GET_INPAYLOAD(ps, pm, fuse_read_in);
- fri->fh = perfuse_get_fh(opc, FREAD);
+ fri->fh = fh;
fri->offset = offset;
fri->size = (uint32_t)MIN(*resid, max_read);
fri->read_flags = 0; /* XXX Unused by libfuse? */
@@ -3184,6 +3180,7 @@ perfuse_node_write2(struct puffs_usermou
struct perfuse_node_data *pnd;
struct vattr *vap;
perfuse_msg_t *pm;
+ uint64_t fh;
struct fuse_write_in *fwi;
struct fuse_write_out *fwo;
size_t data_len;
@@ -3239,6 +3236,8 @@ perfuse_node_write2(struct puffs_usermou
(void *)opc, vap->va_size);
#endif
+ fh = perfuse_get_fh(opc, FWRITE); /* Cannot be VDIR */
+
do {
size_t max_write;
/*
@@ -3262,7 +3261,7 @@ perfuse_node_write2(struct puffs_usermou
*/
pm = ps->ps_new_msg(pu, opc, FUSE_WRITE, payload_len, pcr);
fwi = GET_INPAYLOAD(ps, pm, fuse_write_in);
- fwi->fh = perfuse_get_fh(opc, FWRITE);
+ fwi->fh = fh;
fwi->offset = offset;
fwi->size = (uint32_t)data_len;
fwi->write_flags = (fwi->size % sysconf(_SC_PAGESIZE)) ? 0 : 1;
@@ -3276,7 +3275,7 @@ perfuse_node_write2(struct puffs_usermou
#ifdef PERFUSE_DEBUG
if (perfuse_diagflags & PDF_FH)
DPRINTF("%s: opc = %p, nodeid = 0x%"PRIx64", "
- "fh = 0x%"PRIx64"\n", __func__,
+ "fh = 0x%"PRIx64"\n", __func__,
(void *)opc, pnd->pnd_nodeid, fwi->fh);
#endif
if ((error = xchg_msg(pu, opc, pm,
@@ -3309,7 +3308,7 @@ out:
#ifdef PERFUSE_DEBUG
if (perfuse_diagflags & PDF_RESIZE) {
if (offset > (off_t)vap->va_size)
- DPRINTF("<< %s %p %" PRIu64 " -> %lld\n", __func__,
+ DPRINTF("<< %s %p %" PRIu64 " -> %lld\n", __func__,
(void *)opc, vap->va_size, (long long)offset);
else
DPRINTF("<< %s %p \n", __func__, (void *)opc);
@@ -3671,7 +3670,7 @@ perfuse_node_fallocate(struct puffs_user
pm = ps->ps_new_msg(pu, opc, FUSE_FALLOCATE, sizeof(*fai), NULL);
fai = GET_INPAYLOAD(ps, pm, fuse_fallocate_in);
- fai->fh = perfuse_get_fh(opc, FWRITE);
+ fai->fh = PN_ISDIR(opc) ? FUSE_UNKNOWN_FH : perfuse_get_fh(opc, FWRITE);
fai->offset = off;
fai->length = len;
fai->mode = 0;
Index: src/lib/libperfuse/perfuse.c
diff -u src/lib/libperfuse/perfuse.c:1.40 src/lib/libperfuse/perfuse.c:1.40.6.1
--- src/lib/libperfuse/perfuse.c:1.40 Wed Oct 19 01:30:35 2016
+++ src/lib/libperfuse/perfuse.c Sun Feb 10 13:40:41 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: perfuse.c,v 1.40 2016/10/19 01:30:35 christos Exp $ */
+/* $NetBSD: perfuse.c,v 1.40.6.1 2019/02/10 13:40:41 martin 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.36.8.1
--- src/lib/libperfuse/perfuse_priv.h:1.36 Fri Oct 31 15:12:15 2014
+++ src/lib/libperfuse/perfuse_priv.h Sun Feb 10 13:40:41 2019
@@ -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.36.8.1 2019/02/10 13:40:41 martin 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 *,
