Source-Changes-HG archive

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

[src/trunk]: src/sys Yet more fixes to the pool allocator:



details:   https://anonhg.NetBSD.org/src/rev/e65f6a07a697
branches:  trunk
changeset: 467963:e65f6a07a697
user:      thorpej <thorpej%NetBSD.org@localhost>
date:      Wed Mar 31 23:23:47 1999 +0000

description:
Yet more fixes to the pool allocator:

- Protect userspace from unnecessary header inclusions (as noted on
current-users).

- Some const poisioning.

- GREATLY simplify the locking protocol, and fix potential deadlock
scenarios.  In particular, assume that the back-end page allocator
provides its own locking mechanism (this is currently true for all
such allocators in the NetBSD kernel).  Doing so allows us to simply
use one spin lock for serialized access to all r/w members of the pool
descriptor.  The spin lock is released before calling the back-end
allocator, and re-acquired upon return from it.

- Fix a problem in pr_rmpage() where a data structure was referenced
after it was freed.

- Minor tweak to page manaement.  Migrate both idle and empty pages
to the end of the page list.  As soon as a page becomes un-empty
(by a pool_put()), place it at the head of the page list, and set
curpage to point to it.  This reduces fragmentation as well as the
time required to find a non-empty page as soon as curpage becomes
empty again.

- Use mono_time throughout, and protect access to it w/ splclock().

- In pool_reclaim(), if freeing an idle page would reduce the number
of allocatable items to below the low water mark, don't.

diffstat:

 sys/kern/subr_pool.c |  309 ++++++++++++++++++++++++++++++--------------------
 sys/sys/pool.h       |   49 +++++--
 2 files changed, 217 insertions(+), 141 deletions(-)

diffs (truncated from 826 to 300 lines):

diff -r 699ef3b498b7 -r e65f6a07a697 sys/kern/subr_pool.c
--- a/sys/kern/subr_pool.c      Wed Mar 31 20:45:06 1999 +0000
+++ b/sys/kern/subr_pool.c      Wed Mar 31 23:23:47 1999 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: subr_pool.c,v 1.20 1999/03/31 01:14:06 thorpej Exp $   */
+/*     $NetBSD: subr_pool.c,v 1.21 1999/03/31 23:23:48 thorpej Exp $   */
 
 /*-
  * Copyright (c) 1997, 1999 The NetBSD Foundation, Inc.
@@ -61,7 +61,6 @@
  * headed by `ph_itemlist' in each page header. The memory for building
  * the page list is either taken from the allocated pages themselves (for
  * small pool items) or taken from an internal pool of page headers (`phpool').
- * 
  */
 
 /* List of all pools */
@@ -107,10 +106,13 @@
                *pr_find_pagehead __P((struct pool *, caddr_t));
 static void    pr_rmpage __P((struct pool *, struct pool_item_header *));
 static int     pool_catchup __P((struct pool *));
-static int     pool_prime_page __P((struct pool *, caddr_t));
+static void    pool_prime_page __P((struct pool *, caddr_t));
 static void    *pool_page_alloc __P((unsigned long, int, int));
 static void    pool_page_free __P((void *, unsigned long, int));
 
+#if defined(POOL_DIAGNOSTIC) || defined(DEBUG)
+static void pool_print1 __P((struct pool *, const char *));
+#endif
 
 #ifdef POOL_DIAGNOSTIC
 /*
@@ -173,7 +175,7 @@
        if ((pp->pr_roflags & PR_LOGGING) == 0)
                return;
 
-       pool_print(pp, "printlog");
+       pool_print1(pp, "printlog");
 
        /*
         * Print all entries in this pool's log.
@@ -252,11 +254,6 @@
        pp->pr_npages--;
        pp->pr_npagefree++;
 
-       if ((pp->pr_roflags & PR_PHINPAGE) == 0) {
-               LIST_REMOVE(ph, ph_hashlist);
-               pool_put(&phpool, ph);
-       }
-
        if (pp->pr_curpage == ph) {
                /*
                 * Find a new non-empty page header, if any.
@@ -270,6 +267,11 @@
 
                pp->pr_curpage = ph;
        }
+
+       if ((pp->pr_roflags & PR_PHINPAGE) == 0) {
+               LIST_REMOVE(ph, ph_hashlist);
+               pool_put(&phpool, ph);
+       }
 }
 
 /*
@@ -281,7 +283,7 @@
        u_int   align;
        u_int   ioff;
        int     nitems;
-       char    *wchan;
+       const char *wchan;
        size_t  pagesz;
        void    *(*alloc) __P((unsigned long, int, int));
        void    (*release) __P((void *, unsigned long, int));
@@ -326,7 +328,7 @@
        u_int           align;
        u_int           ioff;
        int             flags;
-       char            *wchan;
+       const char      *wchan;
        size_t          pagesz;
        void            *(*alloc) __P((unsigned long, int, int));
        void            (*release) __P((void *, unsigned long, int));
@@ -445,8 +447,7 @@
        }
 #endif
 
-       simple_lock_init(&pp->pr_lock);
-       lockinit(&pp->pr_resourcelock, PSWP, wchan, 0, 0);
+       simple_lock_init(&pp->pr_slock);
 
        /*
         * Initialize private page header pool if we haven't done so yet.
@@ -526,7 +527,7 @@
        if (curproc == NULL && (flags & PR_WAITOK) != 0)
                panic("pool_get: must have NOWAIT");
 
-       simple_lock(&pp->pr_lock);
+       simple_lock(&pp->pr_slock);
 
  startover:
        /*
@@ -536,7 +537,7 @@
         */
 #ifdef DIAGNOSTIC
        if (pp->pr_nout > pp->pr_hardlimit) {
-               simple_unlock(&pp->pr_lock);
+               simple_unlock(&pp->pr_slock);
                panic("pool_get: %s: crossed hard limit", pp->pr_wchan);
        }
 #endif
@@ -547,9 +548,9 @@
                         * it be?
                         */
                        pp->pr_flags |= PR_WANTED;
-                       simple_unlock(&pp->pr_lock);
+                       simple_unlock(&pp->pr_slock);
                        tsleep((caddr_t)pp, PSWP, pp->pr_wchan, 0);
-                       simple_lock(&pp->pr_lock);
+                       simple_lock(&pp->pr_slock);
                        goto startover;
                }
                if (pp->pr_hardlimit_warning != NULL) {
@@ -567,7 +568,13 @@
                                log(LOG_ERR, "%s\n", pp->pr_hardlimit_warning);
                        }
                }
-               simple_unlock(&pp->pr_lock);
+
+               if (flags & PR_URGENT)
+                       panic("pool_get: urgent");
+
+               pp->pr_nfail++;
+
+               simple_unlock(&pp->pr_slock);
                return (NULL);
        }
 
@@ -579,39 +586,47 @@
         */
        if ((ph = pp->pr_curpage) == NULL) {
                void *v;
-               int lkflags = LK_EXCLUSIVE | LK_INTERLOCK |
-                             ((flags & PR_WAITOK) == 0 ? LK_NOWAIT : 0);
 
 #ifdef DIAGNOSTIC
                if (pp->pr_nitems != 0) {
-                       simple_unlock(&pp->pr_lock);
+                       simple_unlock(&pp->pr_slock);
                        printf("pool_get: %s: curpage NULL, nitems %u\n",
                            pp->pr_wchan, pp->pr_nitems);
                        panic("pool_get: nitems inconsistent\n");
                }
 #endif
 
-               /* Get long-term lock on pool */
-               if (lockmgr(&pp->pr_resourcelock, lkflags, &pp->pr_lock) != 0)
-                       return (NULL);
+               /*
+                * Call the back-end page allocator for more memory.
+                * Release the pool lock, as the back-end page allocator
+                * may block.
+                */
+               simple_unlock(&pp->pr_slock);
+               v = (*pp->pr_alloc)(pp->pr_pagesz, flags, pp->pr_mtype);
+               simple_lock(&pp->pr_slock);
 
-               /* Check if pool became non-empty while we slept */
-               if ((ph = pp->pr_curpage) != NULL)
-                       goto again;
+               if (v == NULL) {
+                       /*
+                        * We were unable to allocate a page, but
+                        * we released the lock during allocation,
+                        * so perhaps items were freed back to the
+                        * pool.  Check for this case.
+                        */
+                       if (pp->pr_curpage != NULL)
+                               goto startover;
 
-               /* Call the page back-end allocator for more memory */
-               v = (*pp->pr_alloc)(pp->pr_pagesz, flags, pp->pr_mtype);
-               if (v == NULL) {
                        if (flags & PR_URGENT)
                                panic("pool_get: urgent");
+
                        if ((flags & PR_WAITOK) == 0) {
                                pp->pr_nfail++;
-                               lockmgr(&pp->pr_resourcelock, LK_RELEASE, NULL);
+                               simple_unlock(&pp->pr_slock);
                                return (NULL);
                        }
 
                        /*
                         * Wait for items to be returned to this pool.
+                        *
                         * XXX: we actually want to wait just until
                         * the page allocator has memory again. Depending
                         * on this pool's usage, we might get stuck here
@@ -620,12 +635,10 @@
                         * XXX: maybe we should wake up once a second and
                         * try again?
                         */
-                       simple_lock(&pp->pr_lock);
-                       (void) lockmgr(&pp->pr_resourcelock, LK_RELEASE, NULL);
                        pp->pr_flags |= PR_WANTED;
-                       simple_unlock(&pp->pr_lock);
+                       simple_unlock(&pp->pr_slock);
                        tsleep((caddr_t)pp, PSWP, pp->pr_wchan, 0);
-                       simple_lock(&pp->pr_lock);
+                       simple_lock(&pp->pr_slock);
                        goto startover;
                }
 
@@ -633,20 +646,17 @@
                pp->pr_npagealloc++;
                pool_prime_page(pp, v);
 
- again:
-               /* Re-acquire pool interlock */
-               simple_lock(&pp->pr_lock);
-               lockmgr(&pp->pr_resourcelock, LK_RELEASE, NULL);
-               
                /* Start the allocation process over. */
                goto startover;
        }
 
-       if ((v = pi = TAILQ_FIRST(&ph->ph_itemlist)) == NULL)
+       if ((v = pi = TAILQ_FIRST(&ph->ph_itemlist)) == NULL) {
+               simple_unlock(&pp->pr_slock);
                panic("pool_get: %s: page empty", pp->pr_wchan);
+       }
 #ifdef DIAGNOSTIC
        if (pp->pr_nitems == 0) {
-               simple_unlock(&pp->pr_lock);
+               simple_unlock(&pp->pr_slock);
                printf("pool_get: %s: items on itemlist, nitems %u\n",
                    pp->pr_wchan, pp->pr_nitems);
                panic("pool_get: nitems inconsistent\n");
@@ -678,17 +688,28 @@
        }
        ph->ph_nmissing++;
        if (TAILQ_FIRST(&ph->ph_itemlist) == NULL) {
+#ifdef DIAGNOSTIC
+               if (ph->ph_nmissing != pp->pr_itemsperpage) {
+                       simple_unlock(&pp->pr_slock);
+                       panic("pool_get: %s: nmissing inconsistent",
+                           pp->pr_wchan);
+               }
+#endif
                /*
                 * Find a new non-empty page header, if any.
                 * Start search from the page head, to increase
                 * the chance for "high water" pages to be freed.
                 *
-                * First, move the now empty page to the head of
-                * the page list.
+                * Migrate empty pages to the end of the list.  This
+                * will speed the update of curpage as pages become
+                * idle.  Empty pages intermingled with idle pages
+                * is no big deal.  As soon as a page becomes un-empty,
+                * it will move back to the head of the list.
                 */
                TAILQ_REMOVE(&pp->pr_pagelist, ph, ph_pagelist);
-               TAILQ_INSERT_HEAD(&pp->pr_pagelist, ph, ph_pagelist);
-               while ((ph = TAILQ_NEXT(ph, ph_pagelist)) != NULL)
+               TAILQ_INSERT_TAIL(&pp->pr_pagelist, ph, ph_pagelist);
+               for (ph = TAILQ_FIRST(&pp->pr_pagelist); ph != NULL;
+                    ph = TAILQ_NEXT(ph, ph_pagelist))
                        if (TAILQ_FIRST(&ph->ph_itemlist) != NULL)
                                break;
 
@@ -709,7 +730,7 @@
                 */
        }
 
-       simple_unlock(&pp->pr_lock);
+       simple_unlock(&pp->pr_slock);
        return (v);
 }
 
@@ -733,10 +754,11 @@
        struct pool_item *pi = v;
        struct pool_item_header *ph;
        caddr_t page;
+       int s;
 
        page = (caddr_t)((u_long)v & pp->pr_pagemask);
 
-       simple_lock(&pp->pr_lock);
+       simple_lock(&pp->pr_slock);
 
        pr_log(pp, v, PRLOG_PUT, file, line);
 
@@ -765,29 +787,51 @@
                pp->pr_flags &= ~PR_WANTED;
                if (ph->ph_nmissing == 0)
                        pp->pr_nidle++;
+               simple_unlock(&pp->pr_slock);
                wakeup((caddr_t)pp);



Home | Main Index | Thread Index | Old Index