Skip to contentSkip to conversation

stuff

PATCH J. Bruce Fields 1 participant 2 messages Apr 24, 2008 – Apr 25, 2008

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

Trond--you mentioned noticing some locks_copy_locks() abuses; is this what you were thinking of?

--b.

commit 07c5abe6899d8ab49368604e64894821d4f60bf9

Author: J. Bruce Fields <[email protected]>
Date:   Thu Apr 24 10:08:22 2008 -0400

locks: don't call ->copy_lock methods on return of conflicting locks

The file_lock structure is used both as a heavy-weight representation of an active lock, with pointers to reference-counted structures, etc., and as a simple container for parameters that describe a file lock.

The conflicting lock returned from __posix_lock_file is an example of the latter; so don't call the filesystem or lock manager callbacks when copying to it. This also saves the need for an unnecessary locks_init_lock in the nfsv4 server.

Thanks to Trond for pointing out the error.

fs/lockd/svclock.c +1 -1
index 1f122c1..4d81553 100644
@@ -632,7 +632,7 @@ nlmsvc_update_deferred_block(struct nlm_block *block, struct file_lock *conf,
 		block->b_flags |= B_TIMED_OUT;
 	if (conf) {
 		if (block->b_fl)
-			locks_copy_lock(block->b_fl, conf);
+			__locks_copy_lock(block->b_fl, conf);
 	}
 }
 
fs/locks.c +2 -2
index 2e0fa66..e1ea2fe 100644
@@ -224,7 +224,7 @@ static void locks_copy_private(struct file_lock *new, struct file_lock *fl)
 /*
  * Initialize a new lock from an existing file_lock structure.
  */
-static void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
+void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
 {
 	new->fl_owner = fl->fl_owner;
 	new->fl_pid = fl->fl_pid;
@@ -833,7 +833,7 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
 			if (!posix_locks_conflict(request, fl))
 				continue;
 			if (conflock)
-				locks_copy_lock(conflock, fl);
+				__locks_copy_lock(conflock, fl);
 			error = -EAGAIN;
 			if (!(request->fl_flags & FL_SLEEP))
 				goto out;
fs/nfsd/nfs4state.c +0 -3
index 55dfdd7..8799b87 100644
@@ -2712,9 +2712,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	* Note: locks.c uses the BKL to protect the inode's lock list.
 	*/
 
-	/* XXX?: Just to divert the locks_release_private at the start of
-	 * locks_copy_lock: */
-	locks_init_lock(&conflock);
 	err = vfs_lock_file(filp, cmd, &file_lock, &conflock);
 	switch (-err) {
 	case 0: /* success! */
include/linux/fs.h +1 -0
index cc2be2c..6556f2f 100644
@@ -973,6 +973,7 @@ extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
 /* fs/locks.c */
 extern void locks_init_lock(struct file_lock *);
 extern void locks_copy_lock(struct file_lock *, struct file_lock *);
+extern void __locks_copy_lock(struct file_lock *, const struct file_lock *);
 extern void locks_remove_posix(struct file *, fl_owner_t);
 extern void locks_remove_flock(struct file *);
 extern void posix_test_lock(struct file *, struct file_lock *);
#1 lore raw

don't call ->copy_lock methods on return of conflicting locks [was: Re: stuff]

And apologies for that subject line. It was a placeholder I forgot to replace....

Quoted 90 lines · J. Bruce Fields

Trond--you mentioned noticing some locks_copy_locks() abuses; is this what you were thinking of?

--b.

commit 07c5abe6899d8ab49368604e64894821d4f60bf9

Author: J. Bruce Fields <[email protected]>
Date:   Thu Apr 24 10:08:22 2008 -0400

locks: don't call ->copy_lock methods on return of conflicting locks

The file_lock structure is used both as a heavy-weight representation of an active lock, with pointers to reference-counted structures, etc., and as a simple container for parameters that describe a file lock.

The conflicting lock returned from __posix_lock_file is an example of the latter; so don't call the filesystem or lock manager callbacks when copying to it. This also saves the need for an unnecessary locks_init_lock in the nfsv4 server.

Thanks to Trond for pointing out the error.

fs/lockd/svclock.c +1 -1
index 1f122c1..4d81553 100644
@@ -632,7 +632,7 @@ nlmsvc_update_deferred_block(struct nlm_block *block, struct file_lock *conf,
 		block->b_flags |= B_TIMED_OUT;
 	if (conf) {
 		if (block->b_fl)
-			locks_copy_lock(block->b_fl, conf);
+			__locks_copy_lock(block->b_fl, conf);
 	}
 }
 
fs/locks.c +2 -2
index 2e0fa66..e1ea2fe 100644
@@ -224,7 +224,7 @@ static void locks_copy_private(struct file_lock *new, struct file_lock *fl)
 /*
  * Initialize a new lock from an existing file_lock structure.
  */
-static void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
+void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
 {
 	new->fl_owner = fl->fl_owner;
 	new->fl_pid = fl->fl_pid;
@@ -833,7 +833,7 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
 			if (!posix_locks_conflict(request, fl))
 				continue;
 			if (conflock)
-				locks_copy_lock(conflock, fl);
+				__locks_copy_lock(conflock, fl);
 			error = -EAGAIN;
 			if (!(request->fl_flags & FL_SLEEP))
 				goto out;
fs/nfsd/nfs4state.c +0 -3
index 55dfdd7..8799b87 100644
@@ -2712,9 +2712,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	* Note: locks.c uses the BKL to protect the inode's lock list.
 	*/
 
-	/* XXX?: Just to divert the locks_release_private at the start of
-	 * locks_copy_lock: */
-	locks_init_lock(&conflock);
 	err = vfs_lock_file(filp, cmd, &file_lock, &conflock);
 	switch (-err) {
 	case 0: /* success! */
include/linux/fs.h +1 -1
index cc2be2c..6556f2f 100644
@@ -973,6 +973,7 @@ extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
 /* fs/locks.c */
 extern void locks_init_lock(struct file_lock *);
 extern void locks_copy_lock(struct file_lock *, struct file_lock *);
+extern void __locks_copy_lock(struct file_lock *, const struct file_lock *);
 extern void locks_remove_posix(struct file *, fl_owner_t);
 extern void locks_remove_flock(struct file *);
 extern void posix_test_lock(struct file *, struct file_lock *);
--

To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html