Re: [PATCH 22/43] CacheFiles: Add a hook to write a single page of data to an inode [ver #46]

From: Nick Piggin
Date: Thu Apr 02 2009 - 11:33:36 EST


On Friday 03 April 2009 01:55:53 David Howells wrote:
> Nick Piggin <nickpiggin@xxxxxxxxxxxx> wrote:
>
> > > Hook the Ext2 and Ext3 operations to the generic implementation.
> >
> > Is this really worthwhile?
>
> I seem to remember being told that I'm not allowed to call such directly (by
> Christoph Hellwig perhaps?), and that I should add an address space op to do
> this.

Hmm, I guess not all filesystems define write_begin/write_end. But if you
only need to use ones that do define them?

>
> > What do you do that write_begin/write_end doesn't?
>
> This code calls write_begin() and write_end().

Yes, you framed the changelog as introducing this new callback because it
allows a highly optimised code that takes advantage of page aligned write.
So I went on a tangent thinking you were going to use it later to avoid
the data copy or something.


> write_begin() and write_end() don't actually update the target page, and don't
> so the osync stuff.
>
> My code assumed file can be passed as NULL to write_begin() and write_end().
> If file is, however, necessary then the backing fs can do something different.
> cachefiles doesn't know.

You are knowingly squashing together fscache and the backing filesystem if
you do something like introduce a flag like PG_owner_priv_2 and disallow the
backing filesystem from reusing it. So at which point you don't have to keep
up illusions about being totally filesystem agnostic.

Many filesystems don't need the file argument to write_begin/write_end.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/