From de7d5a3b6c9ff8429bf046c36b56d3192b75c3da Mon Sep 17 00:00:00 2001 From: "akpm@osdl.org" Date: Sun, 1 May 2005 08:58:39 -0700 Subject: [PATCH] drop_buffers() oops fix In rare situations, drop_buffers() can be called for a page which has buffers, but no ->mapping (it was truncated, but the buffers were left behind because ext3 was still fiddling with them). But if there was an I/O error in a buffer_head, drop_buffers() will try to get at the address_space and will oops. Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/buffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/buffer.c') diff --git a/fs/buffer.c b/fs/buffer.c index 3b12cf947aba..665db84a1f9f 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -2917,7 +2917,7 @@ drop_buffers(struct page *page, struct buffer_head **buffers_to_free) bh = head; do { - if (buffer_write_io_error(bh)) + if (buffer_write_io_error(bh) && page->mapping) set_bit(AS_EIO, &page->mapping->flags); if (buffer_busy(bh)) goto failed; -- cgit v1.2.2 From d59dd4620fb8d6422555a9e2b82a707718e68327 Mon Sep 17 00:00:00 2001 From: "akpm@osdl.org" Date: Sun, 1 May 2005 08:58:47 -0700 Subject: [PATCH] use smp_mb/wmb/rmb where possible Replace a number of memory barriers with smp_ variants. This means we won't take the unnecessary hit on UP machines. Signed-off-by: Anton Blanchard Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/buffer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'fs/buffer.c') diff --git a/fs/buffer.c b/fs/buffer.c index 665db84a1f9f..188365c79204 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -218,7 +218,7 @@ struct super_block *freeze_bdev(struct block_device *bdev) sb = get_super(bdev); if (sb && !(sb->s_flags & MS_RDONLY)) { sb->s_frozen = SB_FREEZE_WRITE; - wmb(); + smp_wmb(); sync_inodes_sb(sb, 0); DQUOT_SYNC(sb); @@ -235,7 +235,7 @@ struct super_block *freeze_bdev(struct block_device *bdev) sync_inodes_sb(sb, 1); sb->s_frozen = SB_FREEZE_TRANS; - wmb(); + smp_wmb(); sync_blockdev(sb->s_bdev); @@ -263,7 +263,7 @@ void thaw_bdev(struct block_device *bdev, struct super_block *sb) if (sb->s_op->unlockfs) sb->s_op->unlockfs(sb); sb->s_frozen = SB_UNFROZEN; - wmb(); + smp_wmb(); wake_up(&sb->s_wait_unfrozen); drop_super(sb); } -- cgit v1.2.2 From cd7619d6bf36564cf54ff7218ef54e558a741913 Mon Sep 17 00:00:00 2001 From: Matt Mackall Date: Sun, 1 May 2005 08:59:01 -0700 Subject: [PATCH] Exterminate PAGE_BUG Remove PAGE_BUG - repalce it with BUG and BUG_ON. Signed-off-by: Matt Mackall Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/buffer.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'fs/buffer.c') diff --git a/fs/buffer.c b/fs/buffer.c index 188365c79204..792cbacbbf41 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -2078,8 +2078,7 @@ int block_read_full_page(struct page *page, get_block_t *get_block) int nr, i; int fully_mapped = 1; - if (!PageLocked(page)) - PAGE_BUG(page); + BUG_ON(!PageLocked(page)); blocksize = 1 << inode->i_blkbits; if (!page_has_buffers(page)) create_empty_buffers(page, blocksize, 0); -- cgit v1.2.2 From 67be2dd1bace0ec7ce2dbc1bba3f8df3d7be597e Mon Sep 17 00:00:00 2001 From: Martin Waitz Date: Sun, 1 May 2005 08:59:26 -0700 Subject: [PATCH] DocBook: fix some descriptions Some KernelDoc descriptions are updated to match the current code. No code changes. Signed-off-by: Martin Waitz Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/buffer.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'fs/buffer.c') diff --git a/fs/buffer.c b/fs/buffer.c index 792cbacbbf41..5f525b3c6d9f 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -774,15 +774,14 @@ repeat: /** * sync_mapping_buffers - write out and wait upon a mapping's "associated" * buffers - * @buffer_mapping - the mapping which backs the buffers' data - * @mapping - the mapping which wants those buffers written + * @mapping: the mapping which wants those buffers written * * Starts I/O against the buffers at mapping->private_list, and waits upon * that I/O. * - * Basically, this is a convenience function for fsync(). @buffer_mapping is - * the blockdev which "owns" the buffers and @mapping is a file or directory - * which needs those buffers to be written for a successful fsync(). + * Basically, this is a convenience function for fsync(). + * @mapping is a file or directory which needs those buffers to be written for + * a successful fsync(). */ int sync_mapping_buffers(struct address_space *mapping) { @@ -1263,6 +1262,7 @@ __getblk_slow(struct block_device *bdev, sector_t block, int size) /** * mark_buffer_dirty - mark a buffer_head as needing writeout + * @bh: the buffer_head to mark dirty * * mark_buffer_dirty() will set the dirty bit against the buffer, then set its * backing page dirty, then tag the page as dirty in its address_space's radix @@ -1501,6 +1501,7 @@ EXPORT_SYMBOL(__breadahead); /** * __bread() - reads a specified block and returns the bh + * @bdev: the block_device to read from * @block: number of block * @size: size (in bytes) to read * -- cgit v1.2.2 From e422fd2c965ad1b0e4eadaabd0adb77e8a93e74e Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli Date: Thu, 5 May 2005 16:15:04 -0700 Subject: [PATCH] avoid -ENOMEM due reclaimable slab caches This makes sure that reclaimable buffer headers and reclaimable inodes are accounted properly during the overcommit checks. Signed-off-by: Andrea Arcangeli Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/buffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/buffer.c') diff --git a/fs/buffer.c b/fs/buffer.c index 5f525b3c6d9f..6ed59497fd4d 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -3115,7 +3115,7 @@ void __init buffer_init(void) bh_cachep = kmem_cache_create("buffer_head", sizeof(struct buffer_head), 0, - SLAB_PANIC, init_buffer_head, NULL); + SLAB_RECLAIM_ACCOUNT|SLAB_PANIC, init_buffer_head, NULL); /* * Limit the bh occupancy to 10% of ZONE_NORMAL -- cgit v1.2.2 From f3ddbdc6267c32223035ea9bb8456a2d86f65ba1 Mon Sep 17 00:00:00 2001 From: Nick Piggin Date: Thu, 5 May 2005 16:15:45 -0700 Subject: [PATCH] fix race in __block_prepare_write Fix a race where __block_prepare_write can leak out an in-flight read against a bh if get_block returns an error. This can lead to the page becoming unlocked while the buffer is locked and the read still in flight. __mpage_writepage BUGs on this condition. BUG sighted on a 2-way Itanium2 system with 16K PAGE_SIZE running fsstress -v -d $DIR/tmp -n 1000 -p 1000 -l 2 where $DIR is a new ext2 filesystem with 4K blocks that is quite small (causing get_block to fail often with -ENOSPC). Signed-off-by: Nick Piggin Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/buffer.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'fs/buffer.c') diff --git a/fs/buffer.c b/fs/buffer.c index 6ed59497fd4d..af7c51ded2e1 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -1953,7 +1953,7 @@ static int __block_prepare_write(struct inode *inode, struct page *page, if (!buffer_mapped(bh)) { err = get_block(inode, block, bh, 1); if (err) - goto out; + break; if (buffer_new(bh)) { clear_buffer_new(bh); unmap_underlying_metadata(bh->b_bdev, @@ -1995,10 +1995,12 @@ static int __block_prepare_write(struct inode *inode, struct page *page, while(wait_bh > wait) { wait_on_buffer(*--wait_bh); if (!buffer_uptodate(*wait_bh)) - return -EIO; + err = -EIO; } - return 0; -out: + if (!err) + return err; + + /* Error case: */ /* * Zero out any newly allocated blocks to avoid exposing stale * data. If BH_New is set, we know that the block was newly -- cgit v1.2.2 From ad576e63e0c8b274a8558b8e05a6d0526e804dc0 Mon Sep 17 00:00:00 2001 From: Nick Piggin Date: Thu, 5 May 2005 16:15:46 -0700 Subject: [PATCH] __block_write_full_page race fix When running fsstress -v -d $DIR/tmp -n 1000 -p 1000 -l 2 on an ext2 filesystem with 1024 byte block size, on SMP i386 with 4096 byte page size over loopback to an image file on a tmpfs filesystem, I would very quickly hit BUG_ON(!buffer_async_write(bh)); in fs/buffer.c:end_buffer_async_write It seems that more than one request would be submitted for a given bh at a time. What would happen is the following: 2 threads doing __mpage_writepages on the same page. Thread 1 - lock the page first, and enter __block_write_full_page. Thread 1 - (eg.) mark_buffer_async_write on the first 2 buffers. Thread 1 - set page writeback, unlock page. Thread 2 - lock page, wait on page writeback Thread 1 - submit_bh on the first 2 buffers. => both requests complete, none of the page buffers are async_write, end_page_writeback is called. Thread 2 - wakes up. enters __block_write_full_page. Thread 2 - mark_buffer_async_write on (eg.) the last buffer Thread 1 - finds the last buffer has async_write set, submit_bh on that. Thread 2 - submit_bh on the last buffer. => oops. So change __block_write_full_page to explicitly keep track of the last bh we need to issue, so we don't touch anything after issuing the last request. Signed-off-by: Nick Piggin Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/buffer.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'fs/buffer.c') diff --git a/fs/buffer.c b/fs/buffer.c index af7c51ded2e1..bc75f2e7b274 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -1751,7 +1751,7 @@ static int __block_write_full_page(struct inode *inode, struct page *page, int err; sector_t block; sector_t last_block; - struct buffer_head *bh, *head; + struct buffer_head *bh, *head, *last_bh = NULL; int nr_underway = 0; BUG_ON(!PageLocked(page)); @@ -1809,7 +1809,6 @@ static int __block_write_full_page(struct inode *inode, struct page *page, } while (bh != head); do { - get_bh(bh); if (!buffer_mapped(bh)) continue; /* @@ -1827,6 +1826,8 @@ static int __block_write_full_page(struct inode *inode, struct page *page, } if (test_clear_buffer_dirty(bh)) { mark_buffer_async_write(bh); + get_bh(bh); + last_bh = bh; } else { unlock_buffer(bh); } @@ -1845,10 +1846,13 @@ static int __block_write_full_page(struct inode *inode, struct page *page, if (buffer_async_write(bh)) { submit_bh(WRITE, bh); nr_underway++; + put_bh(bh); + if (bh == last_bh) + break; } - put_bh(bh); bh = next; } while (bh != head); + bh = head; err = 0; done: @@ -1887,10 +1891,11 @@ recover: bh = head; /* Recovery: lock and submit the mapped buffers */ do { - get_bh(bh); if (buffer_mapped(bh) && buffer_dirty(bh)) { lock_buffer(bh); mark_buffer_async_write(bh); + get_bh(bh); + last_bh = bh; } else { /* * The buffer may have been set dirty during @@ -1909,10 +1914,13 @@ recover: clear_buffer_dirty(bh); submit_bh(WRITE, bh); nr_underway++; + put_bh(bh); + if (bh == last_bh) + break; } - put_bh(bh); bh = next; } while (bh != head); + bh = head; goto done; } -- cgit v1.2.2 From 05937baae9fc27b64bcd4378da7d2b14edf7931c Mon Sep 17 00:00:00 2001 From: Andrew Morton Date: Thu, 5 May 2005 16:15:47 -0700 Subject: [PATCH] __block_write_full_page speedup Remove all those get_bh()'s and put_bh()'s by extending lock_page() to cover the troublesome regions. (get_bh() and put_bh() happen every time whereas contention on a page's lock in there happens basically never). Cc: Nick Piggin Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/buffer.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'fs/buffer.c') diff --git a/fs/buffer.c b/fs/buffer.c index bc75f2e7b274..6f2c3303a443 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -1826,7 +1826,6 @@ static int __block_write_full_page(struct inode *inode, struct page *page, } if (test_clear_buffer_dirty(bh)) { mark_buffer_async_write(bh); - get_bh(bh); last_bh = bh; } else { unlock_buffer(bh); @@ -1839,20 +1838,19 @@ static int __block_write_full_page(struct inode *inode, struct page *page, */ BUG_ON(PageWriteback(page)); set_page_writeback(page); - unlock_page(page); do { struct buffer_head *next = bh->b_this_page; if (buffer_async_write(bh)) { submit_bh(WRITE, bh); nr_underway++; - put_bh(bh); if (bh == last_bh) break; } bh = next; } while (bh != head); bh = head; + unlock_page(page); err = 0; done: @@ -1894,7 +1892,6 @@ recover: if (buffer_mapped(bh) && buffer_dirty(bh)) { lock_buffer(bh); mark_buffer_async_write(bh); - get_bh(bh); last_bh = bh; } else { /* @@ -1914,7 +1911,6 @@ recover: clear_buffer_dirty(bh); submit_bh(WRITE, bh); nr_underway++; - put_bh(bh); if (bh == last_bh) break; } -- cgit v1.2.2 From f0fbd5fc09b20f7ba7bc8c80be33e39925bb38e1 Mon Sep 17 00:00:00 2001 From: Andrew Morton Date: Thu, 5 May 2005 16:15:48 -0700 Subject: [PATCH] __block_write_full_page() simplification The `last_bh' logic probably isn't worth much. In those situations where only the front part of the page is being written out we will save some looping but in the vastly more common case of an all-page writeout if just adds more code. Nick Piggin Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/buffer.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) (limited to 'fs/buffer.c') diff --git a/fs/buffer.c b/fs/buffer.c index 6f2c3303a443..91ace8034bf7 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -1751,7 +1751,7 @@ static int __block_write_full_page(struct inode *inode, struct page *page, int err; sector_t block; sector_t last_block; - struct buffer_head *bh, *head, *last_bh = NULL; + struct buffer_head *bh, *head; int nr_underway = 0; BUG_ON(!PageLocked(page)); @@ -1826,7 +1826,6 @@ static int __block_write_full_page(struct inode *inode, struct page *page, } if (test_clear_buffer_dirty(bh)) { mark_buffer_async_write(bh); - last_bh = bh; } else { unlock_buffer(bh); } @@ -1844,12 +1843,9 @@ static int __block_write_full_page(struct inode *inode, struct page *page, if (buffer_async_write(bh)) { submit_bh(WRITE, bh); nr_underway++; - if (bh == last_bh) - break; } bh = next; } while (bh != head); - bh = head; unlock_page(page); err = 0; @@ -1892,7 +1888,6 @@ recover: if (buffer_mapped(bh) && buffer_dirty(bh)) { lock_buffer(bh); mark_buffer_async_write(bh); - last_bh = bh; } else { /* * The buffer may have been set dirty during @@ -1911,12 +1906,9 @@ recover: clear_buffer_dirty(bh); submit_bh(WRITE, bh); nr_underway++; - if (bh == last_bh) - break; } bh = next; } while (bh != head); - bh = head; goto done; } -- cgit v1.2.2 From 75c96f85845a6707b0f9916cb263cb3584f7d48f Mon Sep 17 00:00:00 2001 From: Adrian Bunk Date: Thu, 5 May 2005 16:16:09 -0700 Subject: [PATCH] make some things static This patch makes some needlessly global identifiers static. Signed-off-by: Adrian Bunk Acked-by: Arjan van de Ven Acked-by: Trond Myklebust Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/buffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/buffer.c') diff --git a/fs/buffer.c b/fs/buffer.c index 91ace8034bf7..6f88dcc6d002 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -1210,7 +1210,7 @@ grow_buffers(struct block_device *bdev, sector_t block, int size) return 1; } -struct buffer_head * +static struct buffer_head * __getblk_slow(struct block_device *bdev, sector_t block, int size) { /* Size must be multiple of hard sectorsize */ -- cgit v1.2.2