Re: The INN/mmap bug

From: Chris Mason (mason@suse.com)
Date: Mon Sep 18 2000 - 12:03:00 EST


--On 09/18/00 12:23:43 -0400 Alexander Viro <viro@math.psu.edu> wrote:

> On Mon, 18 Sep 2000, Chris Mason wrote:
>
>> When reiserfs_get_block is called on the packed data (with create == 1),
>> we unpack it to a full block, so the generic functions can handle
>> dirties/writes from then on. If the data in the page cache isn't up to
>> date, we need to copy the packed data into the page cache so it doesn't
>> get lost in the conversion.
>
> <boggle> why bother with bh in that case? You can do all this work on the
> ->writepage()/->readpage()/->prepare_write()/->commit_write() level.
>

For us, the conversion only needs to be done during prepare_write (readpage
reads directly from the tail into the page). We chose to do it in
get_block because it seemed to fit...prepare_write gives us an offset in
the file, and we give prepare_write a block. Sometimes that means a tail
conversion is involved, sometimes not. prepare_write just doesn't care.

>> This isn't for writing directly to the tail, I'm only doing that in
>> writepage, where I always overwrite the tail data with page data.
>
> Well, duh. Now I can see where the complaints about VFS being
> ext2-friendly at expense of other filesystems come from.
> Sure thing, if
> you bend your code so that it would use helper functions written for
> different layout model... It will hurt. The question being: what for?
> These functions do not belong to VFS API - they are convenient to
> implement the real thing (address_space_operations) for a class of
> filesystems, but that's it.
>
> Look how 'no-holes' filesystems are doing that - yup, different
> layout, different helper functions. It would hurt like hell if we would
> try to do e.g. FAT on get_block() level. So that stuff was done on
> ...page() level, originally in fs/fat/inode.c. When it became obvious that
> there are other filesystems that might share the code - well, into
> fs/buffer.c it went. You are in the same situation.
>

Yes, that's why I posted the reiserfs specific code for writepage and
truncate. If we can pull a generic function out of that, and the work done
for other fragment supporting filesystems, perfect.

> Moreover, right now I'm testing yet another class (fragment
> support). Currently it's UFS only, but ext2 also might eventually win
> from that. Yup, also work on ..._page() level, doing that in
> ufs_get_frag() would be a horrible PITA.
>
> I don't see why are you trying to squeeze everything into
> get_block() - it's not even a method anymore...
>

I'm not trying to put it all into a single get_block call, we have
different get_block funcs for different purposes. What I'm really trying
to do is squeeze into block_prepare_write, as a generic setup function for
file modifications.

The other reason why I convert in get_block is that is when I've got the
most information about the internals of the file, including a path through
the btree to the item that may or may not need converting. In reiserfs,
the only way to find out if the last item of the file is packed is to
search the tree for it, and the same searching code is used if it is a
packed or an unpacked item.

In other words, even if reiserfs_prepare_write didn't use
block_prepare_write, I would probably still convert in a function similar
to reiserfs_get_block.

-chris

-
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:17 EST