Re: [patch] pagecache-2.3.9-H3, bmap & ext2fs cleanup patch

Linus Torvalds (torvalds@transmeta.com)
Sat, 26 Jun 1999 17:46:39 -0700 (PDT)


On Sat, 26 Jun 1999, Ingo Molnar wrote:
>
> only because holes are not really optimized for currently. With the
> buffer_hole() logic i could add 'delayed clearing' of holes as a
> natural extension.

Ingo, stop this. You're not making sense.

> The current code is full of traps and _wrong_
> logic, eg. the partial write case now does:
>
> if (!buffer_mapped(bh)) {
> err = inode->i_op->get_block(inode, block, bh, 1);
> if (err)
> goto out;
> }
>
> if (!buffer_uptodate(bh) && (start_offset ||
> (end_bytes && (i == end_block)))) {
>
> if (buffer_new(bh)) {
> memset(bh->b_data, 0, bh->b_size);
> } else {
> lock_kernel();
> ll_rw_block(READ, 1, &bh);

What's wrong with that?

It makes _perfect_ sense. It makes a whole lot more sense than any of the
alternatives I've seen. It makes two very distinct tests:

- if the mapping does not exist, we create one. If the create fails, we
exit. Clear as a bell, it's obviously correct.

After the above step, we're guaranteed to have a mapping. HOLES DO NOT
EVEN ENTER THE PICTURE ANY MORE! If a filesystem still has a hole by the
time we've required the mapping, that filesystem is obviously broken and
buggy. Nothing subtle about it at all.

- if it's not up-to-date, and we're only going to write a partial buffer,
we need to mark it up-to-date. There are two cases for that: either the
buffer was marked "newly created" or it needs to be read in. Again,
clear as a bell, and obviously correct.

In short: two very simple rules, both of them very obvious indeed.

> this will obviously fail if holes are a little bit different and are not
> uptodate anymore.

"obviously fail" HOW?

The rules are very simple. And HOLE has nothing to do with it. Your hole
defines just made it more complex, imho.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/