Re: [PATCH 25/27] NFS: Configuration and mount option changes toenable local caching on NFS [try #2]

[Trond Myklebust]

On Wed, 2008-01-23 at 17:22 +0000, David Howells wrote:
> Changes to the kernel configuration defintions and to the NFS mount options to
> allow the local caching support added by the previous patch to be enabled.
> 
> Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> ---
> 
>  fs/Kconfig        |    8 ++++++++
>  fs/nfs/client.c   |    2 ++
>  fs/nfs/internal.h |    1 +
>  fs/nfs/super.c    |   14 ++++++++++++++
>  4 files changed, 25 insertions(+), 0 deletions(-)
> 
> 
> diff --git a/fs/Kconfig b/fs/Kconfig
> index e95b11c..39b1981 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -1650,6 +1650,14 @@ config NFS_V4
>  
>  	  If unsure, say N.
>  
> +config NFS_FSCACHE
> +	bool "Provide NFS client caching support (EXPERIMENTAL)"
> +	depends on EXPERIMENTAL
> +	depends on NFS_FS=m && FSCACHE || NFS_FS=y && FSCACHE=y
> +	help
> +	  Say Y here if you want NFS data to be cached locally on disc through
> +	  the general filesystem cache manager
> +
>  config NFS_DIRECTIO
>  	bool "Allow direct I/O on NFS files"
>  	depends on NFS_FS
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index bcdc5d0..92f9b84 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -572,6 +572,7 @@ static int nfs_init_server(struct nfs_server *server,
>  
>  	/* Initialise the client representation from the mount data */
>  	server->flags = data->flags & NFS_MOUNT_FLAGMASK;
> +	server->options = data->options;
>  
>  	if (data->rsize)
>  		server->rsize = nfs_block_size(data->rsize, NULL);
> @@ -931,6 +932,7 @@ static int nfs4_init_server(struct nfs_server *server,
>  	/* Initialise the client representation from the mount data */
>  	server->flags = data->flags & NFS_MOUNT_FLAGMASK;
>  	server->caps |= NFS_CAP_ATOMIC_OPEN;
> +	server->options = data->options;
>  
>  	if (data->rsize)
>  		server->rsize = nfs_block_size(data->rsize, NULL);
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index f3acf48..ef09e00 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -35,6 +35,7 @@ struct nfs_parsed_mount_data {
>  	int			acregmin, acregmax,
>  				acdirmin, acdirmax;
>  	int			namlen;
> +	unsigned int		options;
>  	unsigned int		bsize;
>  	unsigned int		auth_flavor_len;
>  	rpc_authflavor_t	auth_flavors[1];
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 6dd628f..0542550 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -74,6 +74,7 @@ enum {
>  	Opt_acl, Opt_noacl,
>  	Opt_rdirplus, Opt_nordirplus,
>  	Opt_sharecache, Opt_nosharecache,
> +	Opt_fscache, Opt_nofscache,
>  
>  	/* Mount options that take integer arguments */
>  	Opt_port,
> @@ -123,6 +124,8 @@ static match_table_t nfs_mount_option_tokens = {
>  	{ Opt_nordirplus, "nordirplus" },
>  	{ Opt_sharecache, "sharecache" },
>  	{ Opt_nosharecache, "nosharecache" },
> +	{ Opt_fscache, "fsc" },
> +	{ Opt_nofscache, "nofsc" },
>  
>  	{ Opt_port, "port=%u" },
>  	{ Opt_rsize, "rsize=%u" },
> @@ -459,6 +462,8 @@ static void nfs_show_mount_options(struct seq_file *m, struct nfs_server *nfss,
>  	seq_printf(m, ",timeo=%lu", 10U * clp->retrans_timeo / HZ);
>  	seq_printf(m, ",retrans=%u", clp->retrans_count);
>  	seq_printf(m, ",sec=%s", nfs_pseudoflavour_to_name(nfss->client->cl_auth->au_flavor));
> +	if (nfss->options & NFS_OPTION_FSCACHE)
> +		seq_printf(m, ",fsc");
>  }
>  
>  /*
> @@ -697,6 +702,15 @@ static int nfs_parse_mount_options(char *raw,
>  			break;
>  		case Opt_nosharecache:
>  			mnt->flags |= NFS_MOUNT_UNSHARED;
> +			mnt->options &= ~NFS_OPTION_FSCACHE;
> +			break;
> +		case Opt_fscache:
> +			/* sharing is mandatory with fscache */
> +			mnt->options |= NFS_OPTION_FSCACHE;
> +			mnt->flags &= ~NFS_MOUNT_UNSHARED;
> +			break;

This is confusing. If the mount options are incompatible, then it makes
more sense to return an EINVAL instead of silently turning one of them
off.

> +		case Opt_nofscache:
> +			mnt->options &= ~NFS_OPTION_FSCACHE;
>  			break;
>  
>  		case Opt_port:
--
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/