lkml.org 
[lkml]   [2000]   [Sep]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: The INN/mmap bug


On Sun, 17 Sep 2000, Linus Torvalds wrote:

> The basic problem is that right now we have code that does
>
> if (!page->buffers)
> create_empty_buffers(page, inode, inode->i_sb->s_blocksize);
> ...
> if (!buffer_uptodate(bh))
> ll_rw_block(READ, 1, &bh);
>
> And this is WRONG.
>
> If the whole page is up-to-date, we must NOT try to read the buffer in
> from disk, because the in-memory copy is always more up-to-date.

Yep.

> We need to fix the real bug - otherwise anybody doing both write() and
> shared mmap's to the same file is going to be bit by it later on...
>
> The easy fix is probably to do something like
>
> /* Map the buffer */
> if (!buffer_mapped(bh)) {
> ...
> }
> + /* If the page is up-to-date, so is the buffer */
> + if (Page_Uptodate(page))
> + set_bit(BH_Uptodate, &bh->b_state);
>
> /* Ok, now it was _truly_ not uptodate */
> if (!buffer_uptodate(bh))
> ll_rw_block(READ, 1, &bh);
>
> Comments? The above should fix block_write_full_page() automatically, as
> well as fixing the other cases too - and actually improve performance at
> the same time by getting rid of unnecessary re-reads.
>
> Looks fairly simple. It only happens in __block_prepare_write() and in
> block_truncate_page(), so there's just two places to fix.
>
> Can you se anything else wrong?

Looks sane. But I really wonder if we could just do it in
create_page_buffers() if page is up-to-date. OTOH it would require attempt
to map them all. Comments?

That way we restore the bh state if buffer ring is recreated -
IMO somewhat simpler...

-
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/

\
 
 \ /
  Last update: 2005-03-22 12:38    [W:0.064 / U:2.700 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site