kernel - Fix 3:00 a.m. crashes (deadlocks) related to HAMMER VM use
authorMatthew Dillon <dillon@apollo.backplane.com>
Mon, 16 Jan 2012 19:33:33 +0000 (11:33 -0800)
committerMatthew Dillon <dillon@apollo.backplane.com>
Mon, 16 Jan 2012 19:33:33 +0000 (11:33 -0800)
When memory is low and the pageout daemon needs to write things out we still
need to have at least some reserve to perform the supporting operations for
the pageout.  HAMMER is particularly memory intensive and could get into a
situation where insufficient reserve memory was available, deadlocking the
system.

With these changes DragonFly should run stable on systems with as little
as 256M of ram, and possibly a bit lower.

* The getblk/bread/bwrite/etc brelse/bqrelse sequence used to manage buffers
  had several bugs in it that prevented the low memory handling code from
  operating properly.  The b[q]relse() sequence was not properly detecting
  the low memory condition and freeing or caching the underlying VM pages
  (when possible).

* Also change the low memory test used by the buffer cache from 'severe'
  to 'min' in kern/vfs_bio.c.  We may be able to change this back to 'severe'
  at a later date with further testing.  These tests are in brelse(),
  bqrelse(), and vfs_vmio_release().

* Rewrite bio_page_alloc().  It effectively does the same thing that it did
  before but should operate more smoothly.  We also no longer try to recover
  pages from unrelated buffer cache buffers from this function, which could
  lead to deadlocks.  The warning kprintf is now also rate-limited.

* Add a buffer overload test in the hammer dedup ioctl.  A hammer dedup
  could cause a buffer cache deadlock by allowing too many dirty buffers
  to build up.

* Add a VM memory test to the core hammer flusher code that was previously
  only checking for the UNDO meta-data and buffer overload limits.  This
  is now done on a per-record basis and should prevent HAMMER from allocating
  too much memory during a flusher operation when the VM system is already
  too low on memory.

* Add some vm_wait_nominal() calls in critical I/O paths, but make sure we
  do not use these calls in any I/O path used by the HAMMER pageout code.

  Probably the most important path is the vm_object_page_clean*() code
  path, effectively called via either msync() or via the 30-60 second
  system sync.

* Properly bawrite() a buffer in hammer_vop_write() when IO_ASYNC is set
  (which is used by the pageout daemon), otherwise the pageout daemon will
  not be able to directly recover memory in low memory situations when
  paging to a HAMMER file mapped SHARED/RW.

Testing-by: tuxillo, lentferj, ftigeot, dillon
sys/kern/vfs_bio.c
sys/kern/vfs_subr.c
sys/vfs/hammer/hammer_dedup.c
sys/vfs/hammer/hammer_inode.c
sys/vfs/hammer/hammer_reblock.c
sys/vfs/hammer/hammer_vnops.c
sys/vm/vm_object.c
sys/vm/vm_page.c

index 19ec812..a335ebb 100644 (file)
@@ -1172,7 +1172,8 @@ buwrite(struct buf *bp)
 void
 bdirty(struct buf *bp)
 {
-       KASSERT(bp->b_qindex == BQUEUE_NONE, ("bdirty: buffer %p still on queue %d", bp, bp->b_qindex));
+       KASSERT(bp->b_qindex == BQUEUE_NONE,
+               ("bdirty: buffer %p still on queue %d", bp, bp->b_qindex));
        if (bp->b_flags & B_NOCACHE) {
                kprintf("bdirty: clearing B_NOCACHE on buf %p\n", bp);
                bp->b_flags &= ~B_NOCACHE;
@@ -1363,7 +1364,7 @@ brelse(struct buf *bp)
         */
        if ((bp->b_flags & (B_DELWRI | B_LOCKED)) || bp->b_refs) {
                bp->b_flags &= ~B_RELBUF;
-       } else if (vm_page_count_severe()) {
+       } else if (vm_page_count_min(0)) {
                if (LIST_FIRST(&bp->b_dep) != NULL)
                        buf_deallocate(bp);             /* can set B_LOCKED */
                if (bp->b_flags & (B_DELWRI | B_LOCKED))
@@ -1681,7 +1682,7 @@ bqrelse(struct buf *bp)
                bp->b_qindex = (bp->b_flags & B_HEAVY) ?
                               BQUEUE_DIRTY_HW : BQUEUE_DIRTY;
                TAILQ_INSERT_TAIL(&bufqueues[bp->b_qindex], bp, b_freelist);
-       } else if (vm_page_count_severe()) {
+       } else if (vm_page_count_min(0)) {
                /*
                 * We are too low on memory, we have to try to free the
                 * buffer (most importantly: the wired pages making up its
@@ -1745,17 +1746,18 @@ bqdrop(struct buf *bp)
 }
 
 /*
- * vfs_vmio_release:
- *
- *     Return backing pages held by the buffer 'bp' back to the VM system
- *     if possible.  The pages are freed if they are no longer valid or
- *     attempt to free if it was used for direct I/O otherwise they are
- *     sent to the page cache.
- *
- *     Pages that were marked busy are left alone and skipped.
- *
- *     The KVA mapping (b_data) for the underlying pages is removed by
- *     this function.
+ * Return backing pages held by the buffer 'bp' back to the VM system.
+ * This routine is called when the bp is invalidated, released, or
+ * reused.
+ *
+ * The KVA mapping (b_data) for the underlying pages is removed by
+ * this function.
+ *
+ * WARNING! This routine is integral to the low memory critical path
+ *          when a buffer is B_RELBUF'd.  If the system has a severe page
+ *          deficit we need to get the page(s) onto the PQ_FREE or PQ_CACHE
+ *          queues so they can be reused in the current pageout daemon
+ *          pass.
  */
 static void
 vfs_vmio_release(struct buf *bp)
@@ -1767,6 +1769,9 @@ vfs_vmio_release(struct buf *bp)
                m = bp->b_xio.xio_pages[i];
                bp->b_xio.xio_pages[i] = NULL;
 
+               /*
+                * We need to own the page in order to safely unwire it.
+                */
                vm_page_busy_wait(m, FALSE, "vmiopg");
 
                /*
@@ -1794,53 +1799,43 @@ vfs_vmio_release(struct buf *bp)
                        vm_page_unwire(m, 1);
 
                /*
-                * We don't mess with busy pages, it is the responsibility
-                * of the process that busied the pages to deal with them.
-                *
-                * However, the caller may have marked the page invalid and
-                * we must still make sure the page is no longer mapped.
+                * If the wire_count has dropped to 0 we may need to take
+                * further action before unbusying the page
                 */
-               if ((m->flags & PG_BUSY) || (m->busy != 0)) {
-                       vm_page_protect(m, VM_PROT_NONE);
-                       vm_page_wakeup(m);
-                       continue;
-               }
-                       
                if (m->wire_count == 0) {
                        vm_page_flag_clear(m, PG_ZERO);
-                       /*
-                        * Might as well free the page if we can and it has
-                        * no valid data.  We also free the page if the
-                        * buffer was used for direct I/O.
-                        */
-#if 0
-                       if ((bp->b_flags & B_ASYNC) == 0 && !m->valid &&
-                                       m->hold_count == 0) {
-                               vm_page_protect(m, VM_PROT_NONE);
-                               vm_page_free(m);
-                       } else
-#endif
-                       /*
-                        * Cache the page if we are really low on free
-                        * pages.
-                        *
-                        * Also bypass the active and inactive queues
-                        * if B_NOTMETA is set.  This flag is set by HAMMER
-                        * on a regular file buffer when double buffering
-                        * is enabled or on a block device buffer representing
-                        * file data when double buffering is not enabled.
-                        * The flag prevents two copies of the same data from
-                        * being cached for long periods of time.
-                        */
+
                        if (bp->b_flags & B_DIRECT) {
+                               /*
+                                * Attempt to free the page if B_DIRECT is
+                                * set, the caller does not desire the page
+                                * to be cached.
+                                */
                                vm_page_wakeup(m);
                                vm_page_try_to_free(m);
                        } else if ((bp->b_flags & B_NOTMETA) ||
-                                  vm_page_count_severe()) {
+                                  vm_page_count_min(0)) {
+                               /*
+                                * Attempt to move the page to PQ_CACHE
+                                * if B_NOTMETA is set.  This flag is set
+                                * by HAMMER to remove one of the two pages
+                                * present when double buffering is enabled.
+                                *
+                                * Attempt to move the page to PQ_CACHE
+                                * If we have a severe page deficit.  This
+                                * will cause buffer cache operations related
+                                * to pageouts to recycle the related pages
+                                * in order to avoid a low memory deadlock.
+                                */
                                m->act_count = bp->b_act_count;
                                vm_page_wakeup(m);
                                vm_page_try_to_cache(m);
                        } else {
+                               /*
+                                * Nominal case, leave the page on the
+                                * queue the original unwiring placed it on
+                                * (active or inactive).
+                                */
                                m->act_count = bp->b_act_count;
                                vm_page_wakeup(m);
                        }
@@ -2346,12 +2341,14 @@ restart:
        return(bp);
 }
 
+#if 0
 /*
  * This routine is called in an emergency to recover VM pages from the
  * buffer cache by cashing in clean buffers.  The idea is to recover
  * enough pages to be able to satisfy a stuck bio_page_alloc().
  *
- * MPSAFE
+ * XXX Currently not implemented.  This function can wind up deadlocking
+ * against another thread holding one or more of the backing pages busy.
  */
 static int
 recoverbufpages(void)
@@ -2467,6 +2464,7 @@ recoverbufpages(void)
        spin_unlock(&bufqspin);
        return(bytes);
 }
+#endif
 
 /*
  * buf_daemon:
@@ -2479,7 +2477,6 @@ recoverbufpages(void)
  *     of buffers falls below lodirtybuffers, but we will wake up anyone
  *     waiting at the mid-point.
  */
-
 static struct kproc_desc buf_kp = {
        "bufdaemon",
        buf_daemon,
@@ -4526,81 +4523,83 @@ vm_hold_load_pages(struct buf *bp, vm_offset_t from, vm_offset_t to)
 }
 
 /*
- * Allocate pages for a buffer cache buffer.
- *
- * Under extremely severe memory conditions even allocating out of the
- * system reserve can fail.  If this occurs we must allocate out of the
- * interrupt reserve to avoid a deadlock with the pageout daemon.
- *
- * The pageout daemon can run (putpages -> VOP_WRITE -> getblk -> allocbuf).
- * If the buffer cache's vm_page_alloc() fails a vm_wait() can deadlock
- * against the pageout daemon if pages are not freed from other sources.
+ * Allocate a page for a buffer cache buffer.
  *
  * If NULL is returned the caller is expected to retry (typically check if
  * the page already exists on retry before trying to allocate one).
+ *
+ * NOTE! Low-memory handling is dealt with in b[q]relse(), not here.  This
+ *      function will use the system reserve with the hope that the page
+ *      allocations can be returned to PQ_CACHE/PQ_FREE when the caller
+ *      is done with the buffer.
  */
 static
 vm_page_t
 bio_page_alloc(vm_object_t obj, vm_pindex_t pg, int deficit)
 {
+       int vmflags = VM_ALLOC_NORMAL | VM_ALLOC_NULL_OK;
        vm_page_t p;
 
        ASSERT_LWKT_TOKEN_HELD(vm_object_token(obj));
 
        /*
-        * Try a normal allocation, allow use of system reserve.
+        * Try a normal allocation first.
         */
-       p = vm_page_alloc(obj, pg, VM_ALLOC_NORMAL | VM_ALLOC_SYSTEM |
-                                  VM_ALLOC_NULL_OK);
+       p = vm_page_alloc(obj, pg, vmflags);
        if (p)
                return(p);
-
-       /*
-        * The normal allocation failed and we clearly have a page
-        * deficit.  Try to reclaim some clean VM pages directly
-        * from the buffer cache.
-        */
+       if (vm_page_lookup(obj, pg))
+               return(NULL);
        vm_pageout_deficit += deficit;
-       recoverbufpages();
 
        /*
-        * We may have blocked, the caller will know what to do if the
-        * page now exists.
+        * Try again, digging into the system reserve.
+        *
+        * Trying to recover pages from the buffer cache here can deadlock
+        * against other threads trying to busy underlying pages so we
+        * depend on the code in brelse() and bqrelse() to free/cache the
+        * underlying buffer cache pages when memory is low.
         */
-       if (vm_page_lookup(obj, pg)) {
+       if (curthread->td_flags & TDF_SYSTHREAD)
+               vmflags |= VM_ALLOC_SYSTEM | VM_ALLOC_INTERRUPT;
+       else
+               vmflags |= VM_ALLOC_SYSTEM;
+
+       /*recoverbufpages();*/
+       p = vm_page_alloc(obj, pg, vmflags);
+       if (p)
+               return(p);
+       if (vm_page_lookup(obj, pg))
                return(NULL);
-       }
 
        /*
-        * Only system threads can use the interrupt reserve
+        * Wait for memory to free up and try again
         */
-       if ((curthread->td_flags & TDF_SYSTHREAD) == 0) {
-               vm_wait(hz);
-               return(NULL);
-       }
+       if (vm_page_count_severe())
+               ++lowmempgallocs;
+       vm_wait(hz / 20 + 1);
 
+       p = vm_page_alloc(obj, pg, vmflags);
+       if (p)
+               return(p);
+       if (vm_page_lookup(obj, pg))
+               return(NULL);
 
        /*
-        * Allocate and allow use of the interrupt reserve.
-        *
-        * If after all that we still can't allocate a VM page we are
-        * in real trouble, but we slog on anyway hoping that the system
-        * won't deadlock.
+        * Ok, now we are really in trouble.
         */
-       p = vm_page_alloc(obj, pg, VM_ALLOC_NORMAL | VM_ALLOC_SYSTEM |
-                                  VM_ALLOC_INTERRUPT | VM_ALLOC_NULL_OK);
-       if (p) {
-               if (vm_page_count_severe()) {
-                       ++lowmempgallocs;
-                       vm_wait(hz / 20 + 1);
-               }
-       } else if (vm_page_lookup(obj, pg) == NULL) {
-               kprintf("bio_page_alloc: Memory exhausted during bufcache "
-                       "page allocation\n");
-               ++lowmempgfails;
-               vm_wait(hz);
+       {
+               static struct krate biokrate = { .freq = 1 };
+               krateprintf(&biokrate,
+                           "Warning: bio_page_alloc: memory exhausted "
+                           "during bufcache page allocation from %s\n",
+                           curthread->td_comm);
        }
-       return(p);
+       if (curthread->td_flags & TDF_SYSTHREAD)
+               vm_wait(hz / 20 + 1);
+       else
+               vm_wait(hz / 2 + 1);
+       return (NULL);
 }
 
 /*
index 40fcd92..ae1489c 100644 (file)
@@ -865,6 +865,7 @@ vfsync_bp(struct buf *bp, void *data)
                        bawrite(bp);
                }
                waitrunningbufspace();
+               vm_wait_nominal();
                if (info->lazylimit && info->lazycount >= info->lazylimit)
                        error = 1;
                else
index 49a17a4..a481227 100644 (file)
@@ -42,6 +42,7 @@ hammer_ioc_dedup(hammer_transaction_t trans, hammer_inode_t ip,
 {
        struct hammer_cursor cursor1, cursor2;
        int error;
+       int seq;
 
        /*
         * Enforce hammer filesystem version requirements
@@ -175,6 +176,16 @@ done_cursors:
        hammer_done_cursor(&cursor2);
 done_cursor:
        hammer_done_cursor(&cursor1);
+
+       /*
+        * Avoid deadlocking the buffer cache
+        */
+       seq = trans->hmp->flusher.done;
+       while (hammer_flusher_meta_halflimit(trans->hmp) ||
+              hammer_flusher_undo_exhausted(trans, 2)) {
+               hammer_flusher_wait(trans->hmp, seq);
+               seq = hammer_flusher_async_one(trans->hmp);
+       }
        return (error);
 }
 
index 25adbe9..2bacd4e 100644 (file)
@@ -2787,6 +2787,7 @@ hammer_sync_record_callback(hammer_record_t record, void *data)
                                     NULL,
                                     record->leaf.data_len);
        }
+
        for (;;) {
                error = hammer_ip_sync_record_cursor(cursor, record);
                if (error != EDEADLK)
@@ -2814,12 +2815,12 @@ done:
         *
         * WARNING: See warnings in hammer_unlock_cursor() function.
         */
-        if (hammer_flusher_meta_limit(hmp)) {
+        if (hammer_flusher_meta_limit(hmp) ||
+           vm_page_count_severe()) {
                hammer_unlock_cursor(cursor);
                 hammer_flusher_finalize(trans, 0);
                hammer_lock_cursor(cursor);
        }
-
        return(error);
 }
 
index 327c020..0498ace 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008 The DragonFly Project.  All rights reserved.
+ * Copyright (c) 2008-2012 The DragonFly Project.  All rights reserved.
  * 
  * This code is derived from software contributed to The DragonFly Project
  * by Matthew Dillon <dillon@backplane.com>
@@ -30,8 +30,6 @@
  * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
  * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
- * 
- * $DragonFly: src/sys/vfs/hammer/hammer_reblock.c,v 1.34 2008/11/13 02:18:43 dillon Exp $
  */
 /*
  * HAMMER reblocker - This code frees up fragmented physical space
@@ -57,7 +55,7 @@ static int hammer_reblock_int_node(struct hammer_ioc_reblock *reblock,
 
 int
 hammer_ioc_reblock(hammer_transaction_t trans, hammer_inode_t ip,
-              struct hammer_ioc_reblock *reblock)
+                  struct hammer_ioc_reblock *reblock)
 {
        struct hammer_cursor cursor;
        hammer_btree_elm_t elm;
@@ -205,6 +203,7 @@ retry:
                        bwillwrite(HAMMER_XBUFSIZE);
                        hammer_lock_cursor(&cursor);
                }
+               vm_wait_nominal();
 skip:
                if (error == 0) {
                        error = hammer_btree_iterate(&cursor);
index eabe8a9..5155549 100644 (file)
@@ -822,6 +822,11 @@ hammer_vop_write(struct vop_write_args *ap)
                 *        If we queue it immediately the buf could be left
                 *        locked on the device queue for a very long time.
                 *
+                *        However, failing to flush a dirty buffer out when
+                *        issued from the pageout daemon can result in a low
+                *        memory deadlock against bio_page_alloc(), so we
+                *        have to bawrite() on IO_ASYNC as well.
+                *
                 * NOTE!  To avoid degenerate stalls due to mismatched block
                 *        sizes we only honor IO_DIRECT on the write which
                 *        abuts the end of the buffer.  However, we must
@@ -834,6 +839,8 @@ hammer_vop_write(struct vop_write_args *ap)
                        bwrite(bp);
                } else if ((ap->a_ioflag & IO_DIRECT) && endofblk) {
                        bawrite(bp);
+               } else if (ap->a_ioflag & IO_ASYNC) {
+                       bawrite(bp);
                } else {
 #if 0
                if (offset + n == blksize) {
index 68167b9..d99d859 100644 (file)
@@ -1225,6 +1225,7 @@ vm_object_page_clean_pass2(struct vm_page *p, void *data)
         * we raced an object modification.
         */
        vm_object_page_collect_flush(info->object, p, info->pagerflags);
+       vm_wait_nominal();
 done:
        lwkt_yield();
        return(0);
index 6d2ccda..8156a76 100644 (file)
@@ -1764,6 +1764,9 @@ vm_page_free_contig(vm_page_t m, unsigned long size)
 /*
  * Wait for sufficient free memory for nominal heavy memory use kernel
  * operations.
+ *
+ * WARNING!  Be sure never to call this in any vm_pageout code path, which
+ *          will trivially deadlock the system.
  */
 void
 vm_wait_nominal(void)