Revert "ext4: don't release page refs in ext4_end_bio()"
authorLinus Torvalds <torvalds@linux-foundation.org>
Fri, 30 Mar 2012 00:00:56 +0000 (17:00 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Fri, 30 Mar 2012 00:00:56 +0000 (17:00 -0700)
This reverts commit b43d17f319f2c502b17139d1cf70731b2b62c644.

Dave Jones reports that it causes lockups on his laptop, and his debug
output showed a lot of processes hung waiting for page_writeback (or
more commonly - processes hung waiting for a lock that was held during
that writeback wait).

The page_writeback hint made Ted suggest that Dave look at this commit,
and Dave verified that reverting it makes his problems go away.

Ted says:
 "That commit fixes a race which is seen when you write into fallocated
  (and hence uninitialized) disk blocks under *very* heavy memory
  pressure.  Furthermore, although theoretically it could trigger under
  normal direct I/O writes, it only seems to trigger if you are issuing
  a huge number of AIO writes, such that a just-written page can get
  evicted from memory, and then read back into memory, before the
  workqueue has a chance to update the extent tree.

  This race has been around for a little over a year, and no one noticed
  until two months ago; it only happens under fairly exotic conditions,
  and in fact even after trying very hard to create a simple repro under
  lab conditions, we could only reproduce the problem and confirm the
  fix on production servers running MySQL on very fast PCIe-attached
  flash devices.

  Given that Dave was able to hit this problem pretty quickly, if we
  confirm that this commit is at fault, the only reasonable thing to do
  is to revert it IMO."

Reported-and-tested-by: Dave Jones <davej@redhat.com>
Acked-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
fs/ext4/page-io.c

index 74cd1f7f1f888947ac6ef3e2f07c7ec60a66a748..dcdeef169a69811cf995cd34c37bbdcb6d79fd52 100644 (file)
@@ -60,6 +60,7 @@ void ext4_ioend_wait(struct inode *inode)
 static void put_io_page(struct ext4_io_page *io_page)
 {
        if (atomic_dec_and_test(&io_page->p_count)) {
+               end_page_writeback(io_page->p_page);
                put_page(io_page->p_page);
                kmem_cache_free(io_page_cachep, io_page);
        }
@@ -233,9 +234,9 @@ static void ext4_end_bio(struct bio *bio, int error)
                        } while (bh != head);
                }
 
-               if (atomic_read(&io_end->pages[i]->p_count) == 1)
-                       end_page_writeback(io_end->pages[i]->p_page);
+               put_io_page(io_end->pages[i]);
        }
+       io_end->num_io_pages = 0;
        inode = io_end->inode;
 
        if (error) {
@@ -427,8 +428,6 @@ int ext4_bio_write_page(struct ext4_io_submit *io,
         * PageWriteback bit from the page to prevent the system from
         * wedging later on.
         */
-       if (atomic_read(&io_page->p_count) == 1)
-               end_page_writeback(page);
        put_io_page(io_page);
        return ret;
 }