On Mon, 18 Sep 2000, Linus Torvalds wrote:
> It's not a mess at all.
>
> I don't see why you call things "first stage" etc. It's perfectly
> straightforward. There are two bits that matter:
> - buffer uptodate
> - buffer mapped.
first stage == page still not uptodate. second stage == page is already
uptodate and if it reverts to not-uptade state - it's a bug. Why? Because
then all nice logics about "no read requests for mapped pages" goes to
hell. We have such bug, BTW (in generic_file_write()).
> And the state is very clear and unambiguous:
>
> Mapped, Uptodate: regular block
> !Mapped, Uptodate: hole of zeroes
> Mapped, !Uptodate: unread
> !Mapped, !Uptodate: "pending map"
>
> No "several states" at all.
Except that the only reason why check for page being uptodate is correct
is that for such pages we are guaranteed that buffer ring will not be
dropped when page contatins data newer than on disk.
Yes, it works. But I'ld really prefer to avoid these extra reads at all -
if the rule is that buffer is read only once during the whole life of
page the life becomes somewhat easier to explain.
Currently (even with the fixes discussed) we _do_ reread them. Yes, it's
harmless (see above).
> It so happens that we forgot an important part of the "read a buffer"
> equation. The "read a buffer" function is NOT just
>
> static int make_buffer_uptodate(struct buffer_head * bh)
> {
> ll_rw_block(READ, 1, &bh);
> }
>
> but should instead be
>
> static int make_buffer_uptodate(struct page *page, struct buffer_head * bh)
> {
> if (Page_Uptodate(page)) {
^^^^^^^^^^^^^^^^^^^
> set_bit(BH_Uptodate, &bh->b_state);
> return;
> }
> ll_rw_block(READ, 1, &bh);
> }
> Forget about your "stage 1" and "stage 2" complications. They shouldn't
^^^^^^^^^^
> exist.
Erm...
> > 1st stage, uptodate, !mapped hole. Contents is all-zeroes. It may also
> > be a result of failed attempt to map - we
> > have no way to tell.
>
> The "uptodate !mapped" case is entirely clear: it's a hole. No ifs, no
> buts, no nothing.
>
> If a map() failed, that map() will have returned an error, and if
> something turns the buffer uptodate that's a BUG.
Yes. In block_read_full_page(), for one thing. <looking> Oops. Sorry, I
didn't mark it.
-
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:18 EST