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

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

[Wendy Cheng]

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

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

[Steven Whitehouse]

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