Re: [PATCH 3/17] locks: make ->lock release private data before returning in GETLK case

Re: [PATCH 3/17] locks: make ->lock release private data before returning in GETLK case

[Christoph Hellwig]

On Thu, Apr 05, 2007 at 07:40:53PM -0400, J. Bruce Fields wrote:
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index f52cf5c..d557a51 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3019,6 +3019,8 @@ static int _nfs4_proc_getlk(struct nfs4_state *state, int cmd, struct file_lock
>  			status = 0;
>  	}
>  out:
> +	if (request->fl_ops)
> +		request->fl_ops->fl_release_private(request);

Don't we know which fl_release_private instace we'll call and could
do a direct call instead?

Otherwise ok.

Re: [PATCH 3/17] locks: make ->lock release private data before returning in GETLK case

[J. Bruce Fields]

On Fri, Apr 06, 2007 at 06:46:18PM +0100, Christoph Hellwig wrote:
> On Thu, Apr 05, 2007 at 07:40:53PM -0400, J. Bruce Fields wrote:
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index f52cf5c..d557a51 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -3019,6 +3019,8 @@ static int _nfs4_proc_getlk(struct nfs4_state *state, int cmd, struct file_lock
> >  			status = 0;
> >  	}
> >  out:
> > +	if (request->fl_ops)
> > +		request->fl_ops->fl_release_private(request);
> 
> Don't we know which fl_release_private instace we'll call and could
> do a direct call instead?

Yeah.  It's currently private to another file, but that could obviously
be fixed.  The fl_ops check is still required, though.  I could go
either way.

--b.

[PATCH 3/17] locks: make ->lock release private data before returning in GETLK case

[J. Bruce Fields]

From: J. Bruce Fields <bfields@citi.umich.edu> - unquoted

The file_lock argument to ->lock is used to return the conflicting lock
when found.  There's no reason for the filesystem to return any private
information with this conflicting lock, but nfsv4 is.

Fix nfsv4 client, and modify locks.c to stop calling fl_release_private
for it in this case.

Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
---
 fs/locks.c        |    4 ----
 fs/nfs/nfs4proc.c |    2 ++
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 957775b..b07e6e6 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1665,8 +1665,6 @@ int fcntl_getlk(struct file *filp, struct flock __user *l)
 
 	if (filp->f_op && filp->f_op->lock) {
 		error = filp->f_op->lock(filp, F_GETLK, &file_lock);
-		if (file_lock.fl_ops && file_lock.fl_ops->fl_release_private)
-			file_lock.fl_ops->fl_release_private(&file_lock);
 		if (error < 0)
 			goto out;
 		else
@@ -1804,8 +1802,6 @@ int fcntl_getlk64(struct file *filp, struct flock64 __user *l)
 
 	if (filp->f_op && filp->f_op->lock) {
 		error = filp->f_op->lock(filp, F_GETLK, &file_lock);
-		if (file_lock.fl_ops && file_lock.fl_ops->fl_release_private)
-			file_lock.fl_ops->fl_release_private(&file_lock);
 		if (error < 0)
 			goto out;
 		else
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index f52cf5c..d557a51 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3019,6 +3019,8 @@ static int _nfs4_proc_getlk(struct nfs4_state *state, int cmd, struct file_lock
 			status = 0;
 	}
 out:
+	if (request->fl_ops)
+		request->fl_ops->fl_release_private(request);
 	up_read(&clp->cl_sem);
 	return status;
 }
-- 
1.5.0.rc1.gf4b6c