[Cluster-devel] [GFS2 Patch] nfsd readdirplus assertion failure

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

Now added to the GFS2 -nmw git tree. Thanks,

Steve.

On Tue, 2007-02-06 at 03:52 -0500, Wendy Cheng wrote:
> Glock assertion failure found in '07 NFS connectathon. One of the NFSDs
> is doing a "readdirplus" procedure call. It passes the logic into
> gfs2_readdir() where it obtains its directory inode glock. This is then
> followed by filehandle construction that invokes lookup code. It hits
> the assertion failure while trying to obtain the inode glock again
> inside gfs2_drevalidate().  
> 
> This patch bypasses the recursive glock call if caller already holds the
> lock.
> 
> Signed-off-by: S. Wendy Cheng <wcheng@redhat.com>
> 
>  ops_dentry.c |   16 +++++++++++-----
>  1 files changed, 11 insertions(+), 5 deletions(-)
> 
> --- linux-git/fs/gfs2/ops_dentry.c	2007-02-06 01:07:26.000000000 -0500
> +++ linux/fs/gfs2/ops_dentry.c	2007-02-06 01:05:35.000000000 -0500
> @@ -46,6 +46,7 @@ static int gfs2_drevalidate(struct dentr
>  	struct gfs2_inum_host inum;
>  	unsigned int type;
>  	int error;
> +	int had_lock=0;
>  
>  	if (inode && is_bad_inode(inode))
>  		goto invalid;
> @@ -53,9 +54,12 @@ static int gfs2_drevalidate(struct dentr
>  	if (sdp->sd_args.ar_localcaching)
>  		goto valid;
>  
> -	error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED, 0, &d_gh);
> -	if (error)
> -		goto fail;
> +	had_lock = gfs2_glock_is_locked_by_me(dip->i_gl);
> +	if (!had_lock) {
> +		error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED, 0, &d_gh);
> +		if (error)
> +			goto fail;
> +	} 
>  
>  	error = gfs2_dir_search(parent->d_inode, &dentry->d_name, &inum,
> &type);
>  	switch (error) {
> @@ -82,13 +86,15 @@ static int gfs2_drevalidate(struct dentr
>  	}
>  
>  valid_gunlock:
> -	gfs2_glock_dq_uninit(&d_gh);
> +	if (!had_lock)
> +		gfs2_glock_dq_uninit(&d_gh);
>  valid:
>  	dput(parent);
>  	return 1;
>  
>  invalid_gunlock:
> -	gfs2_glock_dq_uninit(&d_gh);
> +	if (!had_lock)
> +		gfs2_glock_dq_uninit(&d_gh);
>  invalid:
>  	if (inode && S_ISDIR(inode->i_mode)) {
>  		if (have_submounts(dentry))
> 
>