On Sun, 17 Sep 2000, Linus Torvalds wrote:
>
>
> On Sun, 17 Sep 2000, Marco d'Itri wrote:
> >
> > I added to block_write_full_page() the debugging code suggested by
> > Alexander Viro:
> >
> > if (inode->i_dev == 0x305 && inode->i_ino == 48991) // Md
> > printk("block_write_full_page: writing page %d, size %Ld\n",
> > page->index, inode->i_size);
> >
> > and I have just been able to trigger the bug (two times):
>
> Bug?
>
> How do you think updates get written back to disk when innd has made
> changes to its in-core file?
>
> Right. With block_write_full_page.
>
> > root@wonderland:vc/2:/var/lib/news$diff -u active active.ok
> > --- active Sun Sep 17 16:09:01 2000
> > +++ active.ok Sun Sep 17 21:03:16 2000
> > @@ -1,11 +1,11 @@
> > control 0000004774 0000004775 y
> > control.cancel 0000021917 0000021889 n
> > junk 0000001777 0000001768 y
> > -fido.ita.ridere 0000014632 0000014600 y
> > +fido.ita.ridere 0000014634 0000014600 y
>
> So? innd got two new articles.
>
> The above is what you want. If the active file wouldn't get updated, you'd
> never see any new articles ever again.
>
> Doesn't look like a bug to me.
It _is_ a bug, unfortunately. Look: he had taken a copy when the thing was
running. I.e. took the copy of in-core page. After that he stopped innd
and that had triggered block_write_full_page(). So far, so good, right?
Except that the page contents didn't reach the disk.
How about the following:
--- buffer.c Mon Sep 18 01:13:42 2000
+++ buffer.c.new Mon Sep 18 01:28:45 2000
@@ -1411,9 +1411,12 @@
*/
static int __block_write_full_page(struct inode *inode, struct page *page, get_block_t *get_block)
{
- int err, i, need_balance_dirty = 0;
+ int err = -EIO, i, need_balance_dirty = 0;
unsigned long block;
struct buffer_head *bh, *head;
+ unsigned long end_block = (inode->i_size+inode->i_sb->s_blocksize-1) >>
+ inode->i_sb->s_blocksize_bits;
+ unsigned offset;
if (!PageLocked(page))
BUG();
@@ -1423,6 +1426,9 @@
head = page->buffers;
block = page->index << (PAGE_CACHE_SHIFT - inode->i_sb->s_blocksize_bits);
+ /* Are we completely out? */
+ if (block > end_block)
+ goto out;
bh = head;
i = 0;
@@ -1451,6 +1457,8 @@
bh = bh->b_this_page;
block++;
+ if (block > end_block)
+ break;
} while (bh != head);
if (need_balance_dirty)
@@ -1823,31 +1831,7 @@
int block_write_full_page(struct page *page, get_block_t *get_block)
{
struct inode *inode = (struct inode*)page->mapping->host;
- unsigned long end_index = inode->i_size >> PAGE_CACHE_SHIFT;
- unsigned offset;
- int err;
-
- /* easy case */
- if (page->index < end_index)
- return __block_write_full_page(inode, page, get_block);
-
- /* things got complicated... */
- offset = inode->i_size & (PAGE_CACHE_SIZE-1);
- /* OK, are we completely out? */
- if (page->index >= end_index+1 || !offset)
- return -EIO;
- /* Sigh... will have to work, then... */
- err = __block_prepare_write(inode, page, 0, offset, get_block);
- if (!err) {
- memset(page_address(page) + offset, 0, PAGE_CACHE_SIZE - offset);
- flush_dcache_page(page);
- __block_commit_write(inode,page,0,offset);
-done:
- kunmap(page);
- return err;
- }
- ClearPageUptodate(page);
- goto done;
+ return __block_write_full_page(inode, page, get_block);
}
int generic_block_bmap(struct address_space *mapping, long block, get_block_t *get_block)
Unlike the current variant it does the right thing with ->b_end_io and is
less intrusive (and shorter, BTW).
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sat Sep 23 2000 - 21:00:16 EST