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/
This archive was generated by hypermail 2b29 : Sat Sep 23 2000 - 21:00:16 EST