Alexander Viro wrote:
>
> On Wed, 13 Sep 2000, Daniel Phillips wrote:
>
> > Alexander Viro wrote:
> > >
> > > On Tue, 12 Sep 2000, Daniel Phillips wrote:
> > > >
> > > > There is a very heavy investment in generic_read/write/mmap - I don't
> > > > want to throw that away. This is a mod to Ext2, and Ext2 uses these
> > >
> > > Oh, but these functions don't need to be modified. Change
> > > address_space_operations and they will be happy.
> >
> > I've been doing that all along and it works fine... but only for
> > generic_file_write, not for generic_file_mmap.
>
> ?
> If your ->readpage() and ->writepage() are working for the last page...
> What's the problem? You don't have to implement your writepage() as a call
> of block_write_full_page(). It just happens to work for UNIX-like and
> no-holes sorts of layout, but there is no reason to do the same thing
> in your case. You probably will find __block_write_full_page() useful
> for handling the case of non-tail pages, but that's about it.
I tried it and you are right. The functionality I had to reproduce
was just:
if (err = <my prepare write>) goto err_out;
memset(page_address(page) + offset, 0, PAGE_CACHE_SIZE - offset);
flush_dcache_page(page);
<my commit write>
kunmap(page);
And the only mysterious are flush_dcache_page and kunmap, both of
which are macros defined for i386 as:
do { } while (0)
(this shouldn't generate any code, so why is it even there?) I guess
these are placeholders for code that has to be there on non-i386
architectures. The point is, it's functionality I have to replicate
in my own block_write_full_page, even though I don't know what it's
for. Fine, it's only a niggle. I'm doing it the way you suggest.
Having perfect block I/O libraries is not essential, just now, today.
:-)
> BTW, before you start exporting the stuff left, right and center - let's
> see how will fs/buffer.c + your helper functions + stuff needed for
> fragments look like. I suspect that we will be able to come up with better
> factorization that way.
I just needed to export one function: create_empty_buffers.
Grab_cache_page will create/find a file page in the correct form for
the page I/O to handle, but if I want to write onto the page I have to
create the buffers. Then I can put the one that holds the tail
fragment into the correct state.
A comment on create_empty_buffers: it takes inode as a parameter and
only uses that to set the b_dev. Shouldn't it just take dev as a
parameter instead of inode?
> > Prepare_write does a lot of stuff associated with actual I/O. I don't
> > need to start a write here, or a read - just get the data onto the
> > page with the buffers in a consistent state and let ext2_get_block
>
> Yes, you do. Think of the case when your data partially covers some block.
> ->prepare_write() is just that:
>
> Do everything that must be done before copying the data into page.
>
> If your page is the tail one - well, you need different things done, but
> write() still fits the "prepare + copy_from_user() + let it roll" pattern.
> So make the first part your ->prepare_write() and the last one -
> ->commit_write() and be done with that.
My ->commit_write operates on merged tails, and what we are talking
about now is the unmerge operation, so it doesn't fit. But you just
helped me find a common factor - read_tail_to_page - consisting of
about 4 lines of code.
I like how it feels using grab_chace_page/create_empty_buffers. It
makes the page cache do a lot of work for me, using nice simple,
efficient operations to set it up. I like the way it lets me put the
state-change operations into the part of the code where they make the
most sense instead of stuffing everything into ext2_get_block and then
getting it to work with a bunch of new flags, variables, etc, that can
all get out of synch.
--
Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
