2010-06-02 13:24:51

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH V2 0/7] Cleancache (was Transcendent Memory): overview

Please give your patches some semi-resonable subject line.

> fs/btrfs/super.c | 2
> fs/buffer.c | 5 +
> fs/ext3/super.c | 2
> fs/ext4/super.c | 2
> fs/mpage.c | 7 +
> fs/ocfs2/super.c | 3
> fs/super.c | 8 +

This is missing out a whole lot of filesystems. Even more so why the
hell do you need hooks into the filesystem?


2010-06-02 16:08:56

by Dan Magenheimer

[permalink] [raw]
Subject: RE: [PATCH V2 0/7] Cleancache (was Transcendent Memory): overview

> From: Christoph Hellwig [mailto:[email protected]]
> Subject: Re: [PATCH V2 0/7] Cleancache (was Transcendent Memory):
> overview

Hi Christophe --

Thanks for your feedback!

> > fs/btrfs/super.c | 2
> > fs/buffer.c | 5 +
> > fs/ext3/super.c | 2
> > fs/ext4/super.c | 2
> > fs/mpage.c | 7 +
> > fs/ocfs2/super.c | 3
> > fs/super.c | 8 +
>
> This is missing out a whole lot of filesystems. Even more so why the
> hell do you need hooks into the filesystem?

Let me rephrase/regroup your question. Let me know if
I missed anything...

1) Why is the VFS layer involved at all?

VFS hooks are necessary to avoid a disk read when a page
is already in cleancache and to maintain coherency (via
cleancache_flush operations) between cleancache, the
page cache, and disk. This very small, very clean set
of hooks (placed by Chris Mason) all compile into
nothingness if cleancache is config'ed off, and turn
into "if (*p == NULL)" if config'ed on but no "backend"
claims cleancache_ops or if an fs doesn't opt-in
(see below).

2) Why do the individual filesystems need to be modified?

Some filesystems are built entirely on top of VFS and
the hooks in VFS are sufficient, so don't require an
fs "cleancache_init" hook; the initial implementation
of cleancache didn't provide this hook. But for some
fs (such as btrfs) the VFS hooks are incomplete and
one or more hooks in the fs-specific code is required.
For some other fs's (such as tmpfs), cleancache may even
be counterproductive.

So it seemed prudent to require an fs to "opt in" to
use cleancache, which requires at least one hook in
any fs.

3) Why are filesystems missing?

Only because they haven't been tested. The existence
proof of four fs's (ext3/ext4/ocfs2/btfrs) should be
sufficient to validate the concept, the opt-in approach
means that untested filesystems are not affected, and
the hooks in the four fs's should serve as examples to
show that it should be very easy to add more fs's in the
future.

> Please give your patches some semi-resonable subject line.

Not sure what you mean... are the subject lines too short?
Or should I leave off the back-reference to Transcendent Memory?
Or please suggest something you think is more reasonable?

Thanks,
Dan