Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/fs/puffs Process flush requests from the file server in ...



details:   https://anonhg.NetBSD.org/src/rev/ecd3779d9faf
branches:  trunk
changeset: 749774:ecd3779d9faf
user:      pooka <pooka%NetBSD.org@localhost>
date:      Mon Dec 07 20:57:55 2009 +0000

description:
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.

diffstat:

 sys/fs/puffs/puffs_msgif.c  |  88 +++++++++++++++++++++++++++++++++++++++-----
 sys/fs/puffs/puffs_sys.h    |  26 ++++++++++++-
 sys/fs/puffs/puffs_vfsops.c |  32 +++++++++++++++-
 3 files changed, 132 insertions(+), 14 deletions(-)

diffs (277 lines):

diff -r fdc7224b57ac -r ecd3779d9faf sys/fs/puffs/puffs_msgif.c
--- a/sys/fs/puffs/puffs_msgif.c        Mon Dec 07 19:57:34 2009 +0000
+++ b/sys/fs/puffs/puffs_msgif.c        Mon Dec 07 20:57:55 2009 +0000
@@ -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
diff -r fdc7224b57ac -r ecd3779d9faf sys/fs/puffs/puffs_sys.h
--- a/sys/fs/puffs/puffs_sys.h  Mon Dec 07 19:57:34 2009 +0000
+++ b/sys/fs/puffs/puffs_sys.h  Mon Dec 07 20:57:55 2009 +0000
@@ -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);
diff -r fdc7224b57ac -r ecd3779d9faf sys/fs/puffs/puffs_vfsops.c
--- a/sys/fs/puffs/puffs_vfsops.c       Mon Dec 07 19:57:34 2009 +0000
+++ b/sys/fs/puffs/puffs_vfsops.c       Mon Dec 07 20:57:55 2009 +0000
@@ -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));



Home | Main Index | Thread Index | Old Index