Module Name: src
Committed By: manu
Date: Thu Aug 16 09:25:44 UTC 2012
Modified Files:
src/lib/libp2k: p2k.c
src/lib/libpuffs: dispatcher.c pnode.c puffs.3 puffs.h puffs_ops.3
Log Message:
Fix regression that has been introduced when the lookup/reclaim race
condition was addressed in libpuffs by counting lookups.
The fix assumes that cookies map to struct puffs_cookie, which has not
been documented as a requirement for filesystems using libpuffs. As an
example, we got burnt by this assumption in libp2k (kern/46734), and
we fixed bit by actually mapping libp2k cookies to struct puffs_node.
It is unlikely, but there may be third party filesystems that use cookies
unmapped to struct puffs_node, and they were left broken for now.
- we introduce a puffs_init() flag PUFFS_FLAG_PNCOOKIE that let filesystems
inform libpuffs that they map cookies to struct puffs_node. Is that flag
is used, the lookup/reclaim race condition fix is enabled. We enable the
flag for libp2k.
- filesystems that use puffs_pn_new() obviouslty use struct puffs_node
and gain PUFFS_FLAG_PNCOOKIE automatically even if they did not specify
it in puffs_init(). This include all our PUFFS filesystem in-tree except
libp2k.
- for filesystems not willing to use struct puffs_node, we introduce a
reclaim2 vnop, which is reclaim with an additionnal lookup count argument.
This vnop let the filesystem implement the lookup/reclaim race fix on
its own.
To generate a diff of this commit:
cvs rdiff -u -r1.56 -r1.57 src/lib/libp2k/p2k.c
cvs rdiff -u -r1.43 -r1.44 src/lib/libpuffs/dispatcher.c
cvs rdiff -u -r1.12 -r1.13 src/lib/libpuffs/pnode.c
cvs rdiff -u -r1.55 -r1.56 src/lib/libpuffs/puffs.3
cvs rdiff -u -r1.123 -r1.124 src/lib/libpuffs/puffs.h
cvs rdiff -u -r1.34 -r1.35 src/lib/libpuffs/puffs_ops.3
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/lib/libp2k/p2k.c
diff -u src/lib/libp2k/p2k.c:1.56 src/lib/libp2k/p2k.c:1.57
--- src/lib/libp2k/p2k.c:1.56 Sun Aug 12 02:51:18 2012
+++ src/lib/libp2k/p2k.c Thu Aug 16 09:25:44 2012
@@ -1,4 +1,4 @@
-/* $NetBSD: p2k.c,v 1.56 2012/08/12 02:51:18 manu Exp $ */
+/* $NetBSD: p2k.c,v 1.57 2012/08/16 09:25:44 manu Exp $ */
/*
* Copyright (c) 2007, 2008, 2009 Antti Kantee. All Rights Reserved.
@@ -358,6 +358,13 @@ p2k_init(uint32_t puffs_flags)
}
}
+ /*
+ * Explicitely tell that our cookies can be treated as
+ * puffs_node, since we never let libpuffs know by
+ * calling call puffs_pn_new()
+ */
+ puffs_flags |= PUFFS_FLAG_PNCOOKIE;
+
p2m = allocp2m();
if (p2m == NULL)
return NULL;
Index: src/lib/libpuffs/dispatcher.c
diff -u src/lib/libpuffs/dispatcher.c:1.43 src/lib/libpuffs/dispatcher.c:1.44
--- src/lib/libpuffs/dispatcher.c:1.43 Fri Aug 10 08:42:10 2012
+++ src/lib/libpuffs/dispatcher.c Thu Aug 16 09:25:43 2012
@@ -1,4 +1,4 @@
-/* $NetBSD: dispatcher.c,v 1.43 2012/08/10 08:42:10 manu Exp $ */
+/* $NetBSD: dispatcher.c,v 1.44 2012/08/16 09:25:43 manu Exp $ */
/*
* Copyright (c) 2006, 2007, 2008 Antti Kantee. All Rights Reserved.
@@ -31,7 +31,7 @@
#include <sys/cdefs.h>
#if !defined(lint)
-__RCSID("$NetBSD: dispatcher.c,v 1.43 2012/08/10 08:42:10 manu Exp $");
+__RCSID("$NetBSD: dispatcher.c,v 1.44 2012/08/16 09:25:43 manu Exp $");
#endif /* !lint */
#include <sys/types.h>
@@ -125,7 +125,7 @@ dispatch(struct puffs_cc *pcc)
struct puffs_req *preq = puffs__framebuf_getdataptr(pcc->pcc_pb);
void *auxbuf; /* help with typecasting */
puffs_cookie_t opcookie;
- int error = 0, buildpath;
+ int error = 0, buildpath, pncookie;
/* XXX: smaller hammer, please */
if ((PUFFSOP_OPCLASS(preq->preq_opclass == PUFFSOP_VFS &&
@@ -145,6 +145,9 @@ dispatch(struct puffs_cc *pcc)
assert((pcc->pcc_flags & PCC_DONE) == 0);
buildpath = pu->pu_flags & PUFFS_FLAG_BUILDPATH;
+ pncookie = pu->pu_flags & PUFFS_FLAG_PNCOOKIE;
+ assert(!buildpath || pncookie);
+
preq->preq_setbacks = 0;
if (pu->pu_flags & PUFFS_FLAG_OPDUMP)
@@ -302,7 +305,7 @@ dispatch(struct puffs_cc *pcc)
}
}
- if (!error) {
+ if (pncookie && !error) {
if (pn == NULL)
pn = PU_CMAP(pu, auxt->pvnr_newnode);
pn->pn_nlookup++;
@@ -349,7 +352,7 @@ dispatch(struct puffs_cc *pcc)
}
}
- if (!error) {
+ if (pncookie && !error) {
if (pn == NULL)
pn = PU_CMAP(pu, auxt->pvnr_newnode);
pn->pn_nlookup++;
@@ -396,7 +399,7 @@ dispatch(struct puffs_cc *pcc)
}
}
- if (!error) {
+ if (pncookie && !error) {
if (pn == NULL)
pn = PU_CMAP(pu, auxt->pvnr_newnode);
pn->pn_nlookup++;
@@ -701,7 +704,7 @@ dispatch(struct puffs_cc *pcc)
}
}
- if (!error) {
+ if (pncookie && !error) {
if (pn == NULL)
pn = PU_CMAP(pu, auxt->pvnr_newnode);
pn->pn_nlookup++;
@@ -766,7 +769,7 @@ dispatch(struct puffs_cc *pcc)
}
}
- if (!error) {
+ if (pncookie && !error) {
if (pn == NULL)
pn = PU_CMAP(pu, auxt->pvnr_newnode);
pn->pn_nlookup++;
@@ -835,6 +838,12 @@ dispatch(struct puffs_cc *pcc)
struct puffs_vnmsg_reclaim *auxt = auxbuf;
struct puffs_node *pn;
+ if (pops->puffs_node_reclaim2 != NULL) {
+ error = pops->puffs_node_reclaim2(pu, opcookie,
+ auxt->pvnr_nlookup);
+ break;
+ }
+
if (pops->puffs_node_reclaim == NULL) {
error = 0;
break;
@@ -847,11 +856,13 @@ dispatch(struct puffs_cc *pcc)
* but before the reply, leaving the kernel
* with a invalid vnode/cookie reference.
*/
- pn = PU_CMAP(pu, opcookie);
- pn->pn_nlookup -= auxt->pvnr_nlookup;
- if (pn->pn_nlookup >= 1) {
- error = 0;
- break;
+ if (pncookie) {
+ pn = PU_CMAP(pu, opcookie);
+ pn->pn_nlookup -= auxt->pvnr_nlookup;
+ if (pn->pn_nlookup >= 1) {
+ error = 0;
+ break;
+ }
}
error = pops->puffs_node_reclaim(pu, opcookie);
Index: src/lib/libpuffs/pnode.c
diff -u src/lib/libpuffs/pnode.c:1.12 src/lib/libpuffs/pnode.c:1.13
--- src/lib/libpuffs/pnode.c:1.12 Wed Apr 18 00:57:22 2012
+++ src/lib/libpuffs/pnode.c Thu Aug 16 09:25:43 2012
@@ -1,4 +1,4 @@
-/* $NetBSD: pnode.c,v 1.12 2012/04/18 00:57:22 manu Exp $ */
+/* $NetBSD: pnode.c,v 1.13 2012/08/16 09:25:43 manu Exp $ */
/*
* Copyright (c) 2006 Antti Kantee. All Rights Reserved.
@@ -27,7 +27,7 @@
#include <sys/cdefs.h>
#if !defined(lint)
-__RCSID("$NetBSD: pnode.c,v 1.12 2012/04/18 00:57:22 manu Exp $");
+__RCSID("$NetBSD: pnode.c,v 1.13 2012/08/16 09:25:43 manu Exp $");
#endif /* !lint */
#include <sys/types.h>
@@ -60,6 +60,8 @@ puffs_pn_new(struct puffs_usermount *pu,
LIST_INSERT_HEAD(&pu->pu_pnodelst, pn, pn_entries);
+ pu->pu_flags |= PUFFS_FLAG_PNCOOKIE;
+
return pn;
}
Index: src/lib/libpuffs/puffs.3
diff -u src/lib/libpuffs/puffs.3:1.55 src/lib/libpuffs/puffs.3:1.56
--- src/lib/libpuffs/puffs.3:1.55 Fri Aug 10 21:00:45 2012
+++ src/lib/libpuffs/puffs.3 Thu Aug 16 09:25:43 2012
@@ -1,4 +1,4 @@
-.\" $NetBSD: puffs.3,v 1.55 2012/08/10 21:00:45 wiz Exp $
+.\" $NetBSD: puffs.3,v 1.56 2012/08/16 09:25:43 manu Exp $
.\"
.\" Copyright (c) 2006, 2007, 2008 Antti Kantee. All rights reserved.
.\"
@@ -235,6 +235,12 @@ to avoid doing a full comparison for eve
the one searched for.
Especially if the file system uses the abovementioned function, it
is a good idea to define this flag.
+.It Dv PUFFS_FLAG_PNCOOKIE
+Tell puffs that cookies map to
+.Vt struct pnode .
+This is automagically set if
+.Fn puffs_pn_new
+is called.
.It Dv PUFFS_KFLAG_CACHE_FS_TTL
Enforce name and attribute caches based on file system-supplied TTL.
In lookup, create, mknod, mkdir, and symlink, the file system must
Index: src/lib/libpuffs/puffs.h
diff -u src/lib/libpuffs/puffs.h:1.123 src/lib/libpuffs/puffs.h:1.124
--- src/lib/libpuffs/puffs.h:1.123 Sat Jul 21 05:17:10 2012
+++ src/lib/libpuffs/puffs.h Thu Aug 16 09:25:44 2012
@@ -1,4 +1,4 @@
-/* $NetBSD: puffs.h,v 1.123 2012/07/21 05:17:10 manu Exp $ */
+/* $NetBSD: puffs.h,v 1.124 2012/08/16 09:25:44 manu Exp $ */
/*
* Copyright (c) 2005, 2006, 2007 Antti Kantee. All Rights Reserved.
@@ -248,8 +248,10 @@ struct puffs_ops {
struct timespec *, int);
int (*puffs_node_write2)(struct puffs_usermount *, puffs_cookie_t,
uint8_t *, off_t, size_t *, const struct puffs_cred *, int, int);
+ int (*puffs_node_reclaim2)(struct puffs_usermount *,
+ puffs_cookie_t, int);
- void *puffs_ops_spare[29];
+ void *puffs_ops_spare[28];
};
typedef int (*pu_pathbuild_fn)(struct puffs_usermount *,
@@ -283,7 +285,8 @@ enum {
#define PUFFS_FLAG_BUILDPATH 0x80000000 /* node paths in pnode */
#define PUFFS_FLAG_OPDUMP 0x40000000 /* dump all operations */
#define PUFFS_FLAG_HASHPATH 0x20000000 /* speedup: hash paths */
-#define PUFFS_FLAG_MASK 0xe0000000
+#define PUFFS_FLAG_PNCOOKIE 0x10000000 /* cookies are pnodes */
+#define PUFFS_FLAG_MASK 0xf0000000
#define PUFFS_FLAG_KERN(a) ((a) & PUFFS_KFLAG_MASK)
#define PUFFS_FLAG_LIB(a) ((a) & PUFFS_FLAG_MASK)
@@ -405,7 +408,9 @@ enum {
struct timespec *, int); \
int fsname##_node_write2(struct puffs_usermount *, \
puffs_cookie_t, uint8_t *, off_t, size_t *, \
- const struct puffs_cred *, int, int);
+ const struct puffs_cred *, int, int); \
+ int fsname##_node_reclaim2(struct puffs_usermount *, \
+ puffs_cookie_t, int);
#define PUFFSOP_INIT(ops) \
Index: src/lib/libpuffs/puffs_ops.3
diff -u src/lib/libpuffs/puffs_ops.3:1.34 src/lib/libpuffs/puffs_ops.3:1.35
--- src/lib/libpuffs/puffs_ops.3:1.34 Sat Jul 28 09:56:09 2012
+++ src/lib/libpuffs/puffs_ops.3 Thu Aug 16 09:25:44 2012
@@ -1,4 +1,4 @@
-.\" $NetBSD: puffs_ops.3,v 1.34 2012/07/28 09:56:09 njoly Exp $
+.\" $NetBSD: puffs_ops.3,v 1.35 2012/08/16 09:25:44 manu Exp $
.\"
.\" Copyright (c) 2007 Antti Kantee. All rights reserved.
.\"
@@ -228,6 +228,10 @@
.Fa "struct puffs_usermount *pu" "puffs_cookie_t opc"
.Fc
.Ft int
+.Fo puffs_node_reclaim2
+.Fa "struct puffs_usermount *pu" "puffs_cookie_t opc" "int nlookup"
+.Fc
+.Ft int
.Fo puffs_node_inactive
.Fa "struct puffs_usermount *pu" "puffs_cookie_t opc"
.Fc
@@ -617,6 +621,8 @@ This is to retain the
open file semantics.
The data may be removed only when
.Fn puffs_node_reclaim
+or
+.Fn puffs_node_reclaim2
is called for the node, as this assures there are no further users.
.It Fn puffs_node_link "pu" "opc" "targ" "pcn"
Create a hard link for the node
@@ -791,6 +797,22 @@ with it may be freed.
In case the file
.Fa opc
has a link count of zero, it may be safely removed now.
+.It Fn puffs_node_reclaim2 "pu" "opc" "nlookup"
+Same as
+.Fn puffs_node_reclaim
+with an addditional argument for the number of lookups that have been done
+on the node (Node creation is counted as a lookup). This can be used by the
+filesystem to avoid a race condition, where the kernel sends a reclaim
+while it does not have received the reply for a lookup. If the filesystem
+tracks lookup count, and compares to
+.Fa nlookup
+it can detect this situation and ignore the reclaim.
+.Pp
+If the filesystem maps cookies to
+.Vt struct puffs_node
+then the framework will do that work, and
+.Fn puffs_node_reclaim
+can be reliabily used without the race condition.
.It Fn puffs_node_abortop "pu" "opc" "pcn"
In case the operation following lookup (e.g., mkdir or remove) is not
executed for some reason, abortop will be issued.
@@ -802,6 +824,8 @@ The node
has lost its last reference in the kernel.
However, the cookie must still remain valid until
.Fn puffs_node_reclaim
+or
+.Fn puffs_node_reclaim2
is called.
.It Fn puffs_setback "pcc" "op"
Issue a "setback" operation which will be handled when the request response
