[OpenAFS-devel] Re: [PATCH] PAG support only

[Christoph Hellwig]

> @@ -166,6 +166,7 @@
>  	if (file->f_op && file->f_op->release)
>  		file->f_op->release(inode, file);
>  	security_file_free(file);
> +	if (file->f_token) vfs_token_put(file->f_token);

Please split this into 2 lines as per Documentation/CodingStyle.  Even better
make vfs_token_put handle a NULL argument.

> diff -uNr linux-2.5.69/fs/open.c linux-2.5.69-cred/fs/open.c
> --- linux-2.5.69/fs/open.c	2003-05-06 15:04:45.000000000 +0100
> +++ linux-2.5.69-cred/fs/open.c	2003-05-13 11:28:08.000000000 +0100
> @@ -46,7 +46,7 @@
>  	struct nameidata nd;
>  	int error;
>  
> -	error = user_path_walk(path, &nd);
> +	error = user_path_walk(path,&nd);

Random whitespace change - and even a wrong one..

> +static inline int is_vfs_token_valid(struct vfs_token *vtoken)
> +{
> +	return !list_empty(&vtoken->link);
> +}

This one is not used - and the name would imply it would be used without taking
a lock and thus racy..

> +static kmem_cache_t *vfs_token_cache;
> +static kmem_cache_t *vfs_pag_cache;

How many of those will be around for a typical AFS client?  I have the vague
feeling the slabs are overkill..

> +	if (pag>0) {
> +		/* join existing PAG */
> +		if (tsk->vfspag->pag &&
> +		    tsk->vfspag->pag==pag)
> +			return pag;

Please try to get your code in conformance with Documentation/CodingStyle.

> +} /* end vfs_pag_put() */