From: Dan Magenheimer on
> From: Nitin Gupta [mailto:ngupta(a)vflare.org]
> Sent: Friday, July 23, 2010 9:05 AM
> To: Dan Magenheimer
> Cc: Christoph Hellwig; akpm(a)linux-foundation.org; Chris Mason;
> viro(a)zeniv.linux.org.uk; adilger(a)sun.com; tytso(a)mit.edu;
> mfasheh(a)suse.com; Joel Becker; matthew(a)wil.cx; linux-
> btrfs(a)vger.kernel.org; linux-kernel(a)vger.kernel.org; linux-
> fsdevel(a)vger.kernel.org; linux-ext4(a)vger.kernel.org; ocfs2-
> devel(a)oss.oracle.com; linux-mm(a)kvack.org; jeremy(a)goop.org;
> JBeulich(a)novell.com; Kurt Hackel; npiggin(a)suse.de; Dave Mccracken;
> riel(a)redhat.com; avi(a)redhat.com; Konrad Wilk
> Subject: Re: [PATCH V3 0/8] Cleancache: overview
>
> On 07/23/2010 08:14 PM, Dan Magenheimer wrote:
> >> From: Christoph Hellwig [mailto:hch(a)infradead.org]
>
>
> >> Also making the ops vector global is just a bad idea.
> >> There is nothing making this sort of caching inherently global.
> >
> > I'm not sure I understand your point, but two very different
> > users of cleancache have been provided, and more will be
> > discussed at the MM summit next month.
> >
> > Do you have a suggestion on how to avoid a global ops
> > vector while still serving the needs of both existing
> > users?
>
> Maybe introduce cleancache_register(struct cleancache_ops *ops)?
> This will allow making cleancache_ops non-global. No value add
> but maybe that's cleaner?

Oh, OK, that seems reasonable.

Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Nitin Gupta on
On 07/23/2010 11:07 PM, Dan Magenheimer wrote:
>> From: Dan Magenheimer
>> Subject: RE: [PATCH V3 0/8] Cleancache: overview
>>
>>> From: Christoph Hellwig [mailto:hch(a)infradead.org]
>>> Subject: Re: [PATCH V3 0/8] Cleancache: overview
>>>
>>> On Fri, Jul 23, 2010 at 06:58:03AM -0700, Dan Magenheimer wrote:
>>>> CHRISTOPH AND ANDREW, if you disagree and your concerns have
>>>> not been resolved, please speak up.
>>
>> Hi Christoph --
>>
>> Thanks very much for the quick (instantaneous?) reply!
>>
>>> Anything that need modification of a normal non-shared fs is utterly
>>> broken and you'll get a clear NAK, so the propsal before is a good
>>> one.
>>
>> Unless/until all filesystems are 100% built on top of VFS,
>> I have to disagree. Abstractions (e.g. VFS) are never perfect.
>
> After thinking about this some more, I can see a way
> to enforce "opt-in" in the cleancache backend without
> any changes to non-generic fs code. I think it's a horrible
> hack and we can try it, but I expect fs maintainers
> would prefer the explicit one-line-patch opt-in.
>
> 1) Cleancache backend maintains a list of "known working"
> filesystems (those that have been tested).

Checks against "known working list" indeed looks horrible.
Isn't there any way to identify pagecache -> disk I/O boundaries
which every filesystem obeys? I'm not yet sure but if this is
doable, then we won't require such hacks.

>
> 2) Nitin's proposed changes pass the *sb as a parameter.
> The string name of the filesystem type is available via
> sb->s_type->name. This can be compared against
> the "known working" list.
>

sb->s_magic could also be used, or better if we can somehow
get rid of these checks :)

> Using the sb pointer as a "handle" requires an extra
> table search on every cleancache get/put/flush,
> and fs/super.c changes are required for fs unmount
> notification anyway (e.g. to call cleancache_flush_fs)
> so I'd prefer to keep the cleancache_poolid addition
> to the sb. I'll assume this is OK since this is in generic
> fs code.
>


I will also try making changes to cleancache so it does not
touch any fs specific code. Though IMHO one liners to fs-code
should really be acceptable but unfortunately this doesn't seem
to be the case. Maybe generic cleancache will have better
chances.

Thanks,
Nitin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Dan Magenheimer on
> > On Fri, Jul 23, 2010 at 06:58:03AM -0700, Dan Magenheimer wrote:
> > > CHRISTOPH AND ANDREW, if you disagree and your concerns have
> > > not been resolved, please speak up.
>
> Hi Christoph --
>
> Thanks very much for the quick (instantaneous?) reply!
>
> > Anything that need modification of a normal non-shared fs is utterly
> > broken and you'll get a clear NAK, so the propsal before is a good
> > one.
>
> No, the per-fs opt-in is very sensible; and its design is
> very minimal.

Not to belabor the point, but maybe the right way to think about
this is:

Cleancache is a new optional feature provided by the VFS layer
that potentially dramatically increases page cache effectiveness
for many workloads in many environments at a negligible cost.

Filesystems that are well-behaved and conform to certain restrictions
can utilize cleancache simply by making a call to cleancache_init_fs
at mount time. Unusual, misbehaving, or poorly layered filesystems
must either add additional hooks and/or undergo extensive additional
testing... or should just not enable the optional cleancache.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Boaz Harrosh on
On 07/24/2010 12:17 AM, Dan Magenheimer wrote:
>>> On Fri, Jul 23, 2010 at 06:58:03AM -0700, Dan Magenheimer wrote:
>>>> CHRISTOPH AND ANDREW, if you disagree and your concerns have
>>>> not been resolved, please speak up.
>>
>> Hi Christoph --
>>
>> Thanks very much for the quick (instantaneous?) reply!
>>
>>> Anything that need modification of a normal non-shared fs is utterly
>>> broken and you'll get a clear NAK, so the propsal before is a good
>>> one.
>>
>> No, the per-fs opt-in is very sensible; and its design is
>> very minimal.
>
> Not to belabor the point, but maybe the right way to think about
> this is:
>
> Cleancache is a new optional feature provided by the VFS layer
> that potentially dramatically increases page cache effectiveness
> for many workloads in many environments at a negligible cost.
>
> Filesystems that are well-behaved and conform to certain restrictions
> can utilize cleancache simply by making a call to cleancache_init_fs
> at mount time. Unusual, misbehaving, or poorly layered filesystems
> must either add additional hooks and/or undergo extensive additional
> testing... or should just not enable the optional cleancache.

OK, So I maintain a filesystem in Kernel. How do I know if my FS
is not "Unusual, misbehaving, or poorly layered"

Thanks
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Dan Magenheimer on
> From: Boaz Harrosh [mailto:bharrosh(a)panasas.com]
> Sent: Tuesday, August 03, 2010 10:23 AM
> To: Dan Magenheimer
> Subject: Re: [PATCH V3 0/8] Cleancache: overview
>
> On 07/24/2010 12:17 AM, Dan Magenheimer wrote:
> >>> On Fri, Jul 23, 2010 at 06:58:03AM -0700, Dan Magenheimer wrote:
> >>>> CHRISTOPH AND ANDREW, if you disagree and your concerns have
> >>>> not been resolved, please speak up.
> >>
> >> Hi Christoph --
> >>
> >> Thanks very much for the quick (instantaneous?) reply!
> >>
> >>> Anything that need modification of a normal non-shared fs is
> utterly
> >>> broken and you'll get a clear NAK, so the propsal before is a good
> >>> one.
> >>
> >> No, the per-fs opt-in is very sensible; and its design is
> >> very minimal.
> >
> > Not to belabor the point, but maybe the right way to think about
> > this is:
> >
> > Cleancache is a new optional feature provided by the VFS layer
> > that potentially dramatically increases page cache effectiveness
> > for many workloads in many environments at a negligible cost.
> >
> > Filesystems that are well-behaved and conform to certain restrictions
> > can utilize cleancache simply by making a call to cleancache_init_fs
> > at mount time. Unusual, misbehaving, or poorly layered filesystems
> > must either add additional hooks and/or undergo extensive additional
> > testing... or should just not enable the optional cleancache.
>
> OK, So I maintain a filesystem in Kernel. How do I know if my FS
> is not "Unusual, misbehaving, or poorly layered"

A reasonable question. I'm not a FS expert so this may not be
a complete answer, but please consider it a start:

- The FS should be block-device-based (e.g. a ram-based FS
such as tmpfs should not enable cleancache)

- To ensure coherency/correctness, the FS must ensure that all
file removal or truncation operations either go through VFS
or add hooks to do the equivalent "flush" operations (e.g.
I started looking at FS-cache-based net FS's and was concerned
there might be problems, dunno for sure)

- To ensure coherency/correctness, inode numbers must be unique
(e.g. no emulating 64-bit inode space on 32-bit inode numbers)

- The FS must call the VFS superblock alloc and deactivate routines
or add hooks to do the equivalent cleancache calls done there.

- To maximize performance, all pages fetched from the FS should
go through the do_mpage_readpage routine or the FS should add
hooks to do the equivalent (e.g. btrfs requires a hook for this)

- Currently, the FS blocksize must be the same as PAGESIZE. This
is not an architectural restriction, but no backends currently
support anything different (e.g. hugetlbfs? should not enable
cleancache)

- A clustered FS should invoke the "shared_init_fs" cleancache
hook to get best performance for some backends.

Does that help?

Thanks,
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/