kernel - Clean up ucred and plimit cache line locality
authorMatthew Dillon <dillon@apollo.backplane.com>
Mon, 16 Oct 2017 00:42:26 +0000 (17:42 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Mon, 16 Oct 2017 18:30:24 +0000 (11:30 -0700)
* Move struct plimit's p_spin and p_refcnt fields into their own
  cacheline.  This structure is massively shared and read often.
  Doing this avoids unnecessary cache line ping-pongs.

* Only use p_spin to modify a resource limit.  Do not use it to
  access the resource limit.

* Integrate plimit's exclusivity flag into p_refcnt.

* Move struct ucred's cr_ref into its own cacheline.  This structure
  is massively shared and read often.  Doing this avoids unnecessary
  cache line ping-pongs.

sys/kern/kern_exit.c
sys/kern/kern_plimit.c
sys/kern/kern_prot.c
sys/sys/resourcevar.h
sys/sys/ucred.h

index 08a2589..cc755cf 100644 (file)
@@ -623,7 +623,13 @@ exit1(int rv)
         *
         * Other substructures are freed from wait().
         */
-       plimit_free(p);
+       if (p->p_limit) {
+               struct plimit *rlimit;
+
+               rlimit = p->p_limit;
+               p->p_limit = NULL;
+               plimit_free(rlimit);
+       }
 
        /*
         * Finally, call machine-dependent code to release as many of the
index 696c77f..c429ef2 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2006 The DragonFly Project.  All rights reserved.
+ * Copyright (c) 2006,2017 The DragonFly Project.  All rights reserved.
  * 
  * This code is derived from software contributed to The DragonFly Project
  * by Matthew Dillon <dillon@backplane.com>
@@ -65,9 +65,7 @@
  * SUCH DAMAGE.
  *
  *     @(#)kern_resource.c     8.5 (Berkeley) 1/21/94
- * $FreeBSD: src/sys/kern/kern_resource.c,v 1.55.2.5 2001/11/03 01:41:08 ps Exp $
  */
-
 #include <sys/resource.h>
 #include <sys/spinlock.h>
 #include <sys/proc.h>
@@ -84,6 +82,8 @@
 
 #include <sys/spinlock2.h>
 
+static MALLOC_DEFINE(M_PLIMIT, "plimit", "resource limits");
+
 static void plimit_copy(struct plimit *olimit, struct plimit *nlimit);
 
 /*
@@ -116,205 +116,130 @@ plimit_init0(struct plimit *limit)
 /*
  * Return a plimit for use by a new forked process given the one
  * contained in the parent process.
- *
- * MPSAFE
  */
 struct plimit *
 plimit_fork(struct proc *p1)
 {
        struct plimit *olimit = p1->p_limit;
-       struct plimit *nlimit = NULL;
-       struct plimit *rlimit;
+       struct plimit *nlimit;
+       uint32_t count;
 
        /*
-        * If we are exclusive (but not threaded-exclusive), but have only
-        * one reference, we can convert the structure to copy-on-write
-        * again.
+        * Try to share the parent's plimit structure.  If we cannot, make
+        * a copy.
         *
-        * If we were threaded but are no longer threaded we can do the same
-        * thing.
+        * NOTE: (count) value is field prior to increment.
         */
-       if (olimit->p_exclusive == 1) {
-               KKASSERT(olimit->p_refcnt == 1);
-               olimit->p_exclusive = 0;
-       } else if (olimit->p_exclusive == 2 && p1->p_nthreads == 1) {
-               KKASSERT(olimit->p_refcnt == 1);
-               olimit->p_exclusive = 0;
-       }
-
-       /*
-        * Take a short-cut that requires limited spin locks.  If we aren't
-        * exclusive we will not be threaded and we can just bump the ref
-        * count.  If that is true and we also have only one ref then there
-        * can be no other accessors.
-        */
-       if (olimit->p_exclusive == 0) {
-               if (olimit->p_refcnt == 1) {
-                       ++olimit->p_refcnt;
+       count = atomic_fetchadd_int(&olimit->p_refcnt, 1);
+       cpu_ccfence();
+       if (count & PLIMITF_EXCLUSIVE) {
+               if ((count & PLIMITF_MASK) == 1 && p1->p_nthreads == 1) {
+                       atomic_clear_int(&olimit->p_refcnt, PLIMITF_EXCLUSIVE);
                } else {
-                       spin_lock(&olimit->p_spin);
-                       ++olimit->p_refcnt;
-                       spin_unlock(&olimit->p_spin);
-               }
-               return(olimit);
-       }
-
-       /*
-        * Full-blown code-up.
-        */
-       nlimit = NULL;
-       spin_lock(&olimit->p_spin);
-
-       for (;;) {
-               if (olimit->p_exclusive == 0) {
-                       ++olimit->p_refcnt;
-                       rlimit = olimit;
-                       break;
-               }
-               if (nlimit) {
+                       nlimit = kmalloc(sizeof(*nlimit), M_PLIMIT, M_WAITOK);
                        plimit_copy(olimit, nlimit);
-                       rlimit = nlimit;
-                       nlimit = NULL;
-                       break;
+                       olimit = nlimit;
                }
-               spin_unlock(&olimit->p_spin);
-               nlimit = kmalloc(sizeof(*nlimit), M_SUBPROC, M_WAITOK);
-               spin_lock(&olimit->p_spin);
        }
-       spin_unlock(&olimit->p_spin);
-       if (nlimit)
-               kfree(nlimit, M_SUBPROC);
-       return(rlimit);
+       return olimit;
 }
 
 /*
  * This routine is called when a new LWP is created for a process.  We
- * must force exclusivity (=2) so p->p_limit remains stable.
+ * must force exclusivity to ensure that p->p_limit remains stable.
  *
  * LWPs share the same process structure so this does not bump refcnt.
  */
 void
 plimit_lwp_fork(struct proc *p)
 {
-       struct plimit *olimit;
+       struct plimit *olimit = p->p_limit;
+       struct plimit *nlimit;
+       uint32_t count;
 
-       for (;;) {
-               olimit = p->p_limit;
-               if (olimit->p_exclusive == 2) {
-                       KKASSERT(olimit->p_refcnt == 1);
-                       break;
-               }
-               if (olimit->p_refcnt == 1) {
-                       olimit->p_exclusive = 2;
-                       break;
+       count = olimit->p_refcnt;
+       cpu_ccfence();
+       if ((count & PLIMITF_EXCLUSIVE) == 0) {
+               if (count != 1) {
+                       nlimit = kmalloc(sizeof(*nlimit), M_PLIMIT, M_WAITOK);
+                       plimit_copy(olimit, nlimit);
+                       p->p_limit = nlimit;
+                       plimit_free(olimit);
+                       olimit = nlimit;
                }
-               plimit_modify(p, -1, NULL);
+               atomic_set_int(&olimit->p_refcnt, PLIMITF_EXCLUSIVE);
        }
 }
 
 /*
- * This routine is called to fixup a proces's p_limit structure prior
+ * This routine is called to fixup a process's p_limit structure prior
  * to it being modified.  If index >= 0 the specified modification is also
  * made.
  *
- * This routine must make the limit structure exclusive.  A later fork
- * will convert it back to copy-on-write if possible.
+ * This routine must make the limit structure exclusive.  If we are threaded,
+ * the structure will already be exclusive.  A later fork will convert it
+ * back to copy-on-write if possible.
  *
  * We can count on p->p_limit being stable since if we had created any
- * threads it will have already been made exclusive (=2).
- *
- * MPSAFE
+ * threads it will have already been made exclusive.
  */
 void
 plimit_modify(struct proc *p, int index, struct rlimit *rlim)
 {
        struct plimit *olimit;
        struct plimit *nlimit;
-       struct plimit *rlimit;
+       uint32_t count;
 
        /*
-        * Shortcut.  If we are not threaded we may be able to trivially
-        * set the structure to exclusive access without needing to acquire
-        * any spinlocks.   The p_limit structure will be stable.
+        * Make exclusive
         */
        olimit = p->p_limit;
-       if (p->p_nthreads == 1) {
-               if (olimit->p_exclusive == 0 && olimit->p_refcnt == 1)
-                       olimit->p_exclusive = 1;
-               if (olimit->p_exclusive) {
-                       if (index >= 0)
-                               p->p_limit->pl_rlimit[index] = *rlim;
-                       return;
+       count = olimit->p_refcnt;
+       cpu_ccfence();
+       if ((count & PLIMITF_EXCLUSIVE) == 0) {
+               if (count != 1) {
+                       nlimit = kmalloc(sizeof(*nlimit), M_PLIMIT, M_WAITOK);
+                       plimit_copy(olimit, nlimit);
+                       p->p_limit = nlimit;
+                       plimit_free(olimit);
+                       olimit = nlimit;
                }
+               atomic_set_int(&olimit->p_refcnt, PLIMITF_EXCLUSIVE);
        }
 
        /*
-        * Full-blown code-up.  Make a copy if we aren't exclusive.  If
-        * we have only one ref we can safely convert the structure to
-        * exclusive without copying.
+        * Make modification
         */
-       nlimit = NULL;
-       spin_lock(&olimit->p_spin);
-
-       for (;;) {
-               if (olimit->p_refcnt == 1) {
-                       if (olimit->p_exclusive == 0)
-                               olimit->p_exclusive = 1;
-                       rlimit = olimit;
-                       break;
-               }
-               KKASSERT(olimit->p_exclusive == 0);
-               if (nlimit) {
-                       plimit_copy(olimit, nlimit);
-                       nlimit->p_exclusive = 1;
-                       p->p_limit = nlimit;
-                       rlimit = nlimit;
-                       nlimit = NULL;
-                       break;
+       if (index >= 0) {
+               if (p->p_nthreads == 1) {
+                       p->p_limit->pl_rlimit[index] = *rlim;
+               } else {
+                       spin_lock(&olimit->p_spin);
+                       p->p_limit->pl_rlimit[index].rlim_cur = rlim->rlim_cur;
+                       p->p_limit->pl_rlimit[index].rlim_max = rlim->rlim_max;
+                       spin_unlock(&olimit->p_spin);
                }
-               spin_unlock(&olimit->p_spin);
-               nlimit = kmalloc(sizeof(*nlimit), M_SUBPROC, M_WAITOK);
-               spin_lock(&olimit->p_spin);
        }
-       if (index >= 0)
-               rlimit->pl_rlimit[index] = *rlim;
-       spin_unlock(&olimit->p_spin);
-       if (nlimit)
-               kfree(nlimit, M_SUBPROC);
 }
 
 /*
  * Destroy a process's plimit structure.
- *
- * MPSAFE
  */
 void
-plimit_free(struct proc *p)
+plimit_free(struct plimit *limit)
 {
-       struct plimit *limit;
+       uint32_t count;
 
-       if ((limit = p->p_limit) != NULL) {
-               p->p_limit = NULL;
+       count = atomic_fetchadd_int(&limit->p_refcnt, -1);
 
-               if (limit->p_refcnt == 1) {
-                       limit->p_refcnt = -999;
-                       kfree(limit, M_SUBPROC);
-               } else {
-                       spin_lock(&limit->p_spin);
-                       if (--limit->p_refcnt == 0) {
-                               spin_unlock(&limit->p_spin);
-                               kfree(limit, M_SUBPROC);
-                       } else {
-                               spin_unlock(&limit->p_spin);
-                       }
-               }
+       if ((count & ~PLIMITF_EXCLUSIVE) == 1) {
+               limit->p_refcnt = -999;
+               kfree(limit, M_PLIMIT);
        }
 }
 
 /*
  * Modify a resource limit (from system call)
- *
- * MPSAFE
  */
 int
 kern_setrlimit(u_int which, struct rlimit *limp)
@@ -438,8 +363,6 @@ kern_setrlimit(u_int which, struct rlimit *limp)
 
 /*
  * The rlimit indexed by which is returned in the second argument.
- *
- * MPSAFE
  */
 int
 kern_getrlimit(u_int which, struct rlimit *limp)
@@ -459,17 +382,14 @@ kern_getrlimit(u_int which, struct rlimit *limp)
                 return (EINVAL);
 
        limit = p->p_limit;
-       spin_lock(&limit->p_spin);
         *limp = p->p_rlimit[which];
-       spin_unlock(&limit->p_spin);
+
         return (0);
 }
 
 /*
  * Determine if the cpu limit has been reached and return an operations
  * code for the caller to perform.
- *
- * MPSAFE
  */
 int
 plimit_testcpulimit(struct plimit *limit, u_int64_t ttime)
@@ -487,7 +407,6 @@ plimit_testcpulimit(struct plimit *limit, u_int64_t ttime)
        if (ttime <= limit->p_cpulimit)
                return(PLIMIT_TESTCPU_OK);
 
-       spin_lock(&limit->p_spin);
        if (ttime > limit->p_cpulimit) {
                rlim = &limit->pl_rlimit[RLIMIT_CPU];
                if (ttime / (rlim_t)1000000 >= rlim->rlim_max + 5)
@@ -497,7 +416,7 @@ plimit_testcpulimit(struct plimit *limit, u_int64_t ttime)
        } else {
                mode = PLIMIT_TESTCPU_OK;
        }
-       spin_unlock(&limit->p_spin);
+
        return(mode);
 }
 
@@ -505,8 +424,6 @@ plimit_testcpulimit(struct plimit *limit, u_int64_t ttime)
  * Helper routine to copy olimit to nlimit and initialize nlimit for
  * use.  nlimit's reference count will be set to 1 and its exclusive bit
  * will be cleared.
- *
- * MPSAFE
  */
 static
 void
@@ -516,7 +433,6 @@ plimit_copy(struct plimit *olimit, struct plimit *nlimit)
 
        spin_init(&nlimit->p_spin, "plimitcopy");
        nlimit->p_refcnt = 1;
-       nlimit->p_exclusive = 0;
 }
 
 /*
index a835084..945a46a 100644 (file)
@@ -1011,7 +1011,7 @@ struct ucred *
 crhold(struct ucred *cr)
 {
        if (cr != NOCRED && cr != FSCRED)
-               atomic_add_int(&cr->cr_ref, 1);
+               atomic_add_long(&cr->cr_ref, 1);
        return(cr);
 }
 
@@ -1028,7 +1028,7 @@ crfree(struct ucred *cr)
 {
        if (cr->cr_ref <= 0)
                panic("Freeing already free credential! %p", cr);
-       if (atomic_fetchadd_int(&cr->cr_ref, -1) == 1) {
+       if (atomic_fetchadd_long(&cr->cr_ref, -1) == 1) {
                /*
                 * Some callers of crget(), such as nfs_statfs(),
                 * allocate a temporary credential, but don't
index a31e811..521378b 100644 (file)
@@ -78,16 +78,24 @@ struct krate {
  * is moderately large but changes infrequently, it is normally
  * shared copy-on-write after forks.
  *
- * Threaded programs force p_exclusive (set it to 2) to prevent the
- * proc->p_limit pointer from changing out from under threaded access.
+ * Threaded programs force PLIMITF_EXCLUSIVE to prevent the proc->p_limit
+ * pointer from changing out from under threaded access.
+ *
+ * p_refcnt can change often, don't force cache mastership changes for
+ * the rest of the (usually read only) data structure.  Place p_refcnt
+ * in its own cache line.
  */
 struct plimit {
        struct  rlimit pl_rlimit[RLIM_NLIMITS];
-       int     p_refcnt;               /* number of references */
-       int     p_exclusive;            /* exclusive to proc due to lwp's */
        rlim_t  p_cpulimit;             /* current cpu limit in usec */
-       struct spinlock p_spin;
-};
+       struct {
+               struct spinlock p_spin; /* protect modifications */
+               uint32_t p_refcnt;      /* refs & exclusivity */
+       } __cachealign;
+} __cachealign;
+
+#define PLIMITF_EXCLUSIVE      0x80000000U
+#define PLIMITF_MASK           0x7FFFFFFFU
 
 #define PLIMIT_TESTCPU_OK      0
 #define PLIMIT_TESTCPU_XCPU    1
@@ -137,7 +145,7 @@ u_int64_t plimit_getadjvalue(int i);
 void plimit_lwp_fork(struct proc *);
 int plimit_testcpulimit(struct plimit *, u_int64_t);
 void plimit_modify(struct proc *, int, struct rlimit *);
-void plimit_free(struct proc *);
+void plimit_free(struct plimit *);
 
 #endif
 
index cbc6778..baf3506 100644 (file)
@@ -51,9 +51,17 @@ struct prison;
  *
  * Please do not inspect cr_uid directly to determine superuserness.
  * Only the priv(9) functions should be used for this.
+ *
+ * NOTE: Creds are accessed a lot, and cr_ref is also adjusted a lot.
+ *      This can ping-pong fields in its cache line that are otherwise
+ *      read-only.  To solve this problem we note that even in really
+ *      busy systems, ucred isn't replicated a whole lot.  So put
+ *      the blasted cr_ref in its own cache line.
  */
 struct ucred {
-       int     cr_ref;                 /* reference count */
+       struct {
+               long    cr_ref;         /* reference count */
+       } __cachealign;
        uid_t   cr_uid;                 /* effective user id */
        short   cr_ngroups;             /* number of groups */
        gid_t   cr_groups[NGROUPS];     /* groups */
@@ -64,7 +72,8 @@ struct ucred {
        uid_t   cr_svuid;               /* Saved effective user id. */
        gid_t   cr_rgid;                /* Real group id. */
        gid_t   cr_svgid;               /* Saved effective group id. */
-};
+} __cachealign;
+
 #define cr_gid cr_groups[0]
 #define NOCRED ((struct ucred *)0)     /* no credential available */
 #define FSCRED ((struct ucred *)-1)    /* filesystem credential */