Re: [PATCH V3 0/8] Cleancache: overview

[Nitin Gupta <ngupta@vflare.org>]

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@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