Module Name: src
Committed By: pooka
Date: Mon Dec 7 20:57:55 UTC 2009
Modified Files:
src/sys/fs/puffs: puffs_msgif.c puffs_sys.h puffs_vfsops.c
Log Message:
Process flush requests from the file server in a separate thread
context. This fixes a long-standing but seldomly seen deadlock,
where the kernel was holding pages busy (due to e.g. readahead
request) while waiting for the server to respond, and the server
made a callback into the kernel asking to invalidate those pages.
... or, well, theoretically fixes, since I didn't have any reliable
way of repeating the deadlock and I think I saw it only twice.
To generate a diff of this commit:
cvs rdiff -u -r1.75 -r1.76 src/sys/fs/puffs/puffs_msgif.c
cvs rdiff -u -r1.72 -r1.73 src/sys/fs/puffs/puffs_sys.h
cvs rdiff -u -r1.83 -r1.84 src/sys/fs/puffs/puffs_vfsops.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/fs/puffs/puffs_msgif.c
diff -u src/sys/fs/puffs/puffs_msgif.c:1.75 src/sys/fs/puffs/puffs_msgif.c:1.76
--- src/sys/fs/puffs/puffs_msgif.c:1.75 Mon Dec 7 15:51:52 2009
+++ src/sys/fs/puffs/puffs_msgif.c Mon Dec 7 20:57:55 2009
@@ -1,4 +1,4 @@
-/* $NetBSD: puffs_msgif.c,v 1.75 2009/12/07 15:51:52 pooka Exp $ */
+/* $NetBSD: puffs_msgif.c,v 1.76 2009/12/07 20:57:55 pooka Exp $ */
/*
* Copyright (c) 2005, 2006, 2007 Antti Kantee. All Rights Reserved.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: puffs_msgif.c,v 1.75 2009/12/07 15:51:52 pooka Exp $");
+__KERNEL_RCSID(0, "$NetBSD: puffs_msgif.c,v 1.76 2009/12/07 20:57:55 pooka Exp $");
#include <sys/param.h>
#include <sys/atomic.h>
@@ -759,10 +759,7 @@
voff_t offlo, offhi;
int rv, flags = 0;
- if (pf->pf_req.preq_pth.pth_framelen != sizeof(struct puffs_flush)) {
- rv = EINVAL;
- goto out;
- }
+ KASSERT(pf->pf_req.preq_pth.pth_framelen == sizeof(struct puffs_flush));
/* XXX: slurry */
if (pf->pf_op == PUFFS_INVAL_NAMECACHE_ALL) {
@@ -845,9 +842,8 @@
{
struct puffs_mount *pmp = this;
struct puffs_req *preq = (struct puffs_req *)pth;
- int rv = 0;
+ struct puffs_sopreq *psopr;
- /* XXX: need to send error to userspace */
if (pth->pth_framelen < sizeof(struct puffs_req)) {
puffs_msg_sendresp(pmp, preq, EINVAL); /* E2SMALL */
return 0;
@@ -859,17 +855,87 @@
DPRINTF(("dispatch: vn/vfs message 0x%x\n", preq->preq_optype));
puffsop_msg(pmp, preq);
break;
- case PUFFSOP_FLUSH:
+ case PUFFSOP_FLUSH: /* process in sop thread */
+ {
+ struct puffs_flush *pf;
+
DPRINTF(("dispatch: flush 0x%x\n", preq->preq_optype));
- puffsop_flush(pmp, (struct puffs_flush *)preq);
+
+ if (preq->preq_pth.pth_framelen != sizeof(struct puffs_flush)) {
+ puffs_msg_sendresp(pmp, preq, EINVAL); /* E2SMALL */
+ break;
+ }
+ pf = (struct puffs_flush *)preq;
+
+ psopr = kmem_alloc(sizeof(*psopr), KM_SLEEP);
+ memcpy(&psopr->psopr_pf, pf, sizeof(*pf));
+ psopr->psopr_sopreq = PUFFS_SOPREQ_FLUSH;
+
+ mutex_enter(&pmp->pmp_sopmtx);
+ TAILQ_INSERT_TAIL(&pmp->pmp_sopreqs, psopr, psopr_entries);
+ cv_signal(&pmp->pmp_sopcv);
+ mutex_exit(&pmp->pmp_sopmtx);
break;
+ }
default:
DPRINTF(("dispatch: invalid class 0x%x\n", preq->preq_opclass));
puffs_msg_sendresp(pmp, preq, EOPNOTSUPP);
break;
}
- return rv;
+ return 0;
+}
+
+/*
+ * Work loop for thread processing all ops from server which
+ * cannot safely be handled in caller context. This includes
+ * everything which might need a lock currently "held" by the file
+ * server, i.e. a long-term kernel lock which will be released only
+ * once the file server acknowledges a request
+ */
+void
+puffs_sop_thread(void *arg)
+{
+ struct puffs_mount *pmp = arg;
+ struct puffs_sopreq *psopr;
+ bool keeprunning;
+
+ mutex_enter(&pmp->pmp_sopmtx);
+ for (keeprunning = true; keeprunning; ) {
+ while ((psopr = TAILQ_FIRST(&pmp->pmp_sopreqs)) == NULL)
+ cv_wait(&pmp->pmp_sopcv, &pmp->pmp_sopmtx);
+ TAILQ_REMOVE(&pmp->pmp_sopreqs, psopr, psopr_entries);
+ mutex_exit(&pmp->pmp_sopmtx);
+
+ switch (psopr->psopr_sopreq) {
+ case PUFFS_SOPREQ_EXIT:
+ keeprunning = false;
+ break;
+ case PUFFS_SOPREQ_FLUSH:
+ puffsop_flush(pmp, &psopr->psopr_pf);
+ break;
+ }
+
+ kmem_free(psopr, sizeof(*psopr));
+ mutex_enter(&pmp->pmp_sopmtx);
+ }
+
+ /*
+ * Purge remaining ops. could send error, but that is highly
+ * unlikely to reach the caller.
+ */
+ while ((psopr = TAILQ_FIRST(&pmp->pmp_sopreqs)) != NULL) {
+ TAILQ_REMOVE(&pmp->pmp_sopreqs, psopr, psopr_entries);
+ mutex_exit(&pmp->pmp_sopmtx);
+ kmem_free(psopr, sizeof(*psopr));
+ mutex_enter(&pmp->pmp_sopmtx);
+ }
+
+ pmp->pmp_sopthrcount--;
+ cv_signal(&pmp->pmp_sopcv);
+ mutex_exit(&pmp->pmp_sopmtx); /* not allowed to access fs after this */
+
+ kthread_exit(0);
}
int
Index: src/sys/fs/puffs/puffs_sys.h
diff -u src/sys/fs/puffs/puffs_sys.h:1.72 src/sys/fs/puffs/puffs_sys.h:1.73
--- src/sys/fs/puffs/puffs_sys.h:1.72 Thu Nov 5 19:42:44 2009
+++ src/sys/fs/puffs/puffs_sys.h Mon Dec 7 20:57:55 2009
@@ -1,4 +1,4 @@
-/* $NetBSD: puffs_sys.h,v 1.72 2009/11/05 19:42:44 pooka Exp $ */
+/* $NetBSD: puffs_sys.h,v 1.73 2009/12/07 20:57:55 pooka Exp $ */
/*
* Copyright (c) 2005, 2006 Antti Kantee. All Rights Reserved.
@@ -98,6 +98,23 @@
LIST_ENTRY(puffs_newcookie) pnc_entries;
};
+enum puffs_sopreqtype {
+ PUFFS_SOPREQ_EXIT,
+ PUFFS_SOPREQ_FLUSH,
+};
+
+struct puffs_sopreq {
+ union {
+ struct puffs_req preq;
+ struct puffs_flush pf;
+ } psopr_u;
+
+ enum puffs_sopreqtype psopr_sopreq;
+ TAILQ_ENTRY(puffs_sopreq) psopr_entries;
+};
+#define psopr_preq psopr_u.preq
+#define psopr_pf psopr_u.pf
+
TAILQ_HEAD(puffs_wq, puffs_msgpark);
LIST_HEAD(puffs_node_hashlist, puffs_node);
struct puffs_mount {
@@ -143,6 +160,11 @@
void *pmp_curopaq;
uint64_t pmp_nextmsgid;
+
+ kmutex_t pmp_sopmtx;
+ kcondvar_t pmp_sopcv;
+ int pmp_sopthrcount;
+ TAILQ_HEAD(, puffs_sopreq) pmp_sopreqs;
};
#define PUFFSTAT_BEFOREINIT 0
@@ -194,6 +216,8 @@
int puffs_msgmem_alloc(size_t, struct puffs_msgpark **, void **, int);
void puffs_msgmem_release(struct puffs_msgpark *);
+void puffs_sop_thread(void *);
+
void puffs_msg_setfaf(struct puffs_msgpark *);
void puffs_msg_setdelta(struct puffs_msgpark *, size_t);
void puffs_msg_setinfo(struct puffs_msgpark *, int, int, puffs_cookie_t);
Index: src/sys/fs/puffs/puffs_vfsops.c
diff -u src/sys/fs/puffs/puffs_vfsops.c:1.83 src/sys/fs/puffs/puffs_vfsops.c:1.84
--- src/sys/fs/puffs/puffs_vfsops.c:1.83 Thu Nov 5 19:42:44 2009
+++ src/sys/fs/puffs/puffs_vfsops.c Mon Dec 7 20:57:55 2009
@@ -1,4 +1,4 @@
-/* $NetBSD: puffs_vfsops.c,v 1.83 2009/11/05 19:42:44 pooka Exp $ */
+/* $NetBSD: puffs_vfsops.c,v 1.84 2009/12/07 20:57:55 pooka Exp $ */
/*
* Copyright (c) 2005, 2006 Antti Kantee. All Rights Reserved.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: puffs_vfsops.c,v 1.83 2009/11/05 19:42:44 pooka Exp $");
+__KERNEL_RCSID(0, "$NetBSD: puffs_vfsops.c,v 1.84 2009/12/07 20:57:55 pooka Exp $");
#include <sys/param.h>
#include <sys/mount.h>
@@ -42,6 +42,7 @@
#include <sys/kauth.h>
#include <sys/proc.h>
#include <sys/module.h>
+#include <sys/kthread.h>
#include <dev/putter/putter_sys.h>
@@ -250,11 +251,19 @@
pmp->pmp_root_rdev = args->pa_root_rdev;
mutex_init(&pmp->pmp_lock, MUTEX_DEFAULT, IPL_NONE);
+ mutex_init(&pmp->pmp_sopmtx, MUTEX_DEFAULT, IPL_NONE);
cv_init(&pmp->pmp_msg_waiter_cv, "puffsget");
cv_init(&pmp->pmp_refcount_cv, "puffsref");
cv_init(&pmp->pmp_unmounting_cv, "puffsum");
+ cv_init(&pmp->pmp_sopcv, "puffsop");
TAILQ_INIT(&pmp->pmp_msg_touser);
TAILQ_INIT(&pmp->pmp_msg_replywait);
+ TAILQ_INIT(&pmp->pmp_sopreqs);
+
+ if ((error = kthread_create(PRI_NONE, KTHREAD_MPSAFE, NULL,
+ puffs_sop_thread, pmp, NULL, "puffsop")) != 0)
+ goto out;
+ pmp->pmp_sopthrcount = 1;
DPRINTF(("puffs_mount: mount point at %p, puffs specific at %p\n",
mp, MPTOPUFFSMP(mp)));
@@ -262,6 +271,8 @@
vfs_getnewfsid(mp);
out:
+ if (error && pmp && pmp->pmp_pi)
+ putter_detach(pmp->pmp_pi);
if (error && pmp && pmp->pmp_pnodehash)
kmem_free(pmp->pmp_pnodehash, BUCKETALLOC(pmp->pmp_npnodehash));
if (error && pmp)
@@ -336,6 +347,8 @@
* screw what userland thinks and just die.
*/
if (error == 0 || force) {
+ struct puffs_sopreq *psopr;
+
/* tell waiters & other resources to go unwait themselves */
puffs_userdead(pmp);
putter_detach(pmp->pmp_pi);
@@ -352,11 +365,26 @@
cv_wait(&pmp->pmp_refcount_cv, &pmp->pmp_lock);
mutex_exit(&pmp->pmp_lock);
+ /*
+ * Release kernel thread now that there is nothing
+ * it would be wanting to lock.
+ */
+ psopr = kmem_alloc(sizeof(*psopr), KM_SLEEP);
+ psopr->psopr_sopreq = PUFFS_SOPREQ_EXIT;
+ mutex_enter(&pmp->pmp_sopmtx);
+ TAILQ_INSERT_TAIL(&pmp->pmp_sopreqs, psopr, psopr_entries);
+ cv_signal(&pmp->pmp_sopcv);
+ while (pmp->pmp_sopthrcount > 0)
+ cv_wait(&pmp->pmp_sopcv, &pmp->pmp_sopmtx);
+ mutex_exit(&pmp->pmp_sopmtx);
+
/* free resources now that we hopefully have no waiters left */
cv_destroy(&pmp->pmp_unmounting_cv);
cv_destroy(&pmp->pmp_refcount_cv);
cv_destroy(&pmp->pmp_msg_waiter_cv);
+ cv_destroy(&pmp->pmp_sopcv);
mutex_destroy(&pmp->pmp_lock);
+ mutex_destroy(&pmp->pmp_sopmtx);
kmem_free(pmp->pmp_pnodehash, BUCKETALLOC(pmp->pmp_npnodehash));
kmem_free(pmp, sizeof(struct puffs_mount));
