Re: [PATCH 1/3] cifs: allow fsc mount option only if CONFIG_CIFS_FSCACHE is set

[Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>]

On 11/27/2010 09:21 AM, Steve French wrote:
> 
> 
> On Fri, Nov 26, 2010 at 9:50 PM, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
> <mailto:smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
> 
>     As an alternative - if fsc is stable and no defects and doesn't
>     increase the code size appreciably ... what are the drawbacks of
>     turning it on unconditionally?
> 
> 
> ie always configure it, we still only enable it on a mount when fsc is
> specified on mount
> �

I'm not aware of any open issues w.r.t cifs FS-Cache (except the
problems this patchset addresses) but I'm not sure how much testing it
has undergone really (except for me running all my tests with fsc on for
sometime and a few openSUSE users). The code size is not really much of
a problem.

The strict cache patches accounting for fscache might also provide more
testing ground. So, I think keeping it as non-default atleast for one
more release would be good before we make it as a config default.

Thanks,

>     On Wed, Nov 24, 2010 at 6:19 AM, Suresh Jayaraman
>     <sjayaraman-l3A5Bk7waGM@public.gmane.org <mailto:sjayaraman-l3A5Bk7waGM@public.gmane.org>> wrote:
> 
>         Currently, it is possible to specify 'fsc' mount option even if
>         CONFIG_CIFS_FSCACHE has not been set. The option is being
>         ignored silently
>         while the user fscache functionality to work. Fix this by
>         raising error when
>         the CONFIG option is not set.
> 
> 
>         Reported-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
>         <mailto:jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>>
>         Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org
>         <mailto:sjayaraman-l3A5Bk7waGM@public.gmane.org>>
>         ---
>         �fs/cifs/connect.c | � �5 +++++
>         �1 files changed, 5 insertions(+), 0 deletions(-)
> 
>         diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>         index 251a17c..32fa4d9 100644
>         --- a/fs/cifs/connect.c
>         +++ b/fs/cifs/connect.c
>         @@ -1352,6 +1352,11 @@ cifs_parse_mount_options(char *options,
>         const char *devname,
>         � � � � � � � � � � � � � � � �"supported. Instead set "
>         � � � � � � � � � � � � � � � �"/proc/fs/cifs/LookupCacheEnabled
>         to 0\n");
>         � � � � � � � �} else if (strnicmp(data, "fsc", 3) == 0) {
>         +#ifndef CONFIG_CIFS_FSCACHE
>         + � � � � � � � � � � � cERROR(1, "FS-Cache support needs
>         CONFIG_CIFS_FSCACHE"
>         + � � � � � � � � � � � � � � � � "kernel config option set");
>         + � � � � � � � � � � � return 1;
>         +#endif
>         � � � � � � � � � � � �vol->fsc = true;
>         � � � � � � � �} else if (strnicmp(data, "mfsymlinks", 10) == 0) {
>         � � � � � � � � � � � �vol->mfsymlinks = true;
> 
> 
> 
> 
>     -- 
>     Thanks,
> 
>     Steve
> 
> 
> 
> 
> -- 
> Thanks,
> 
> Steve


-- 
Suresh Jayaraman