Home » Mailing list archives » linux-fsdevel » 2011 » January » 4

[PATCH 2/4] nfsd4: use a single struct file for delegations

Previous thread: Re: [PATCH] Update atime from future. by Steven Whitehouse on Monday, January 3, 2011 - 9:41 am. (1 message)

Next thread: [PATCH v2 0/5] add new ioctls to do metadata readahead in btrfs by Shaohua Li on Monday, January 3, 2011 - 10:40 pm. (2 messages)
[view original message]
From: J. Bruce Fields
Subject: remove lease callbacks
Date: Monday, January 3, 2011 - 8:06 pm

A couple lock manager calls used by the lease code look unnecessary to
me; the following patches remove them.  I'm planning on submitting them
with the nfsd patches for 2.6.38, barring any objections.

--b.

--


[ message continues ]
[view original message]
From: J. Bruce Fields
Subject: [PATCH 2/4] nfsd4: use a single struct file for delegations
Date: Monday, January 3, 2011 - 8:06 pm

When we converted to sharing struct filess between nfs4 opens I went too
far and also used the same mechanism for delegations.  But keeping
a reference to the struct file ensures it will outlast the lease, and
allows us to remove the lease with the same file as we added it.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c |   10 +++++-----
 fs/nfsd/state.h     |    1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 2e44ad2..cbe1b81 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -230,7 +230,8 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_stateid *stp, struct svc_f
 	dp->dl_client = clp;
 	get_nfs4_file(fp);
 	dp->dl_file = fp;
-	nfs4_file_get_access(fp, O_RDONLY);
+	dp->dl_vfs_file = find_readable_file(fp);
+	get_file(dp->dl_vfs_file);
 	dp->dl_flock = NULL;
 	dp->dl_type = type;
 	dp->dl_stateid.si_boot = boot_time;
@@ -252,6 +253,7 @@ nfs4_put_delegation(struct nfs4_delegation *dp)
 	if (atomic_dec_and_test(&dp->dl_count)) {
 		dprintk("NFSD: freeing dp %p\n",dp);
 		put_nfs4_file(dp->dl_file);
+		fput(dp->dl_vfs_file);
 		kmem_cache_free(deleg_slab, dp);
 		num_delegations--;
 	}
@@ -265,12 +267,10 @@ nfs4_put_delegation(struct nfs4_delegation *dp)
 static void
 nfs4_close_delegation(struct nfs4_delegation *dp)
 {
-	struct file *filp = find_readable_file(dp->dl_file);
-
 	dprintk("NFSD: close_delegation dp %p\n",dp);
+	/* XXX: do we even need this check?: */
 	if (dp->dl_flock)
-		vfs_setlease(filp, F_UNLCK, &dp->dl_flock);
-	nfs4_file_put_access(dp->dl_file, O_RDONLY);
+		vfs_setlease(dp->dl_vfs_file, F_UNLCK, &dp->dl_flock);
 }
 
 /* Called under the state lock. */
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 39adc27..84b2302 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -81,6 +81,7 @@ struct nfs4_delegation {
 	atomic_t		dl_count;       /* ref count */
 	struct nfs4_client	*dl_client;
 	struct ...
[ message continues ]
[view original message]
From: J. Bruce Fields
Subject: [PATCH 3/4] locks: eliminate fl_mylease callback
Date: Monday, January 3, 2011 - 8:06 pm

The nfs server only supports read delegations for now, so we don't care
how conflicts are determined.  All we care is that unlocks are
recognized as matching the leases they are meant to remove.  After the
last patch, a comparison of struct files will work for that purpose.  So
we no longer need this callback.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/locks.c          |    8 +-------
 fs/nfsd/nfs4state.c |   21 +--------------------
 include/linux/fs.h  |    1 -
 3 files changed, 2 insertions(+), 28 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 8729347..5cb6506 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -444,15 +444,9 @@ static void lease_release_private_callback(struct file_lock *fl)
 	fl->fl_file->f_owner.signum = 0;
 }
 
-static int lease_mylease_callback(struct file_lock *fl, struct file_lock *try)
-{
-	return fl->fl_file == try->fl_file;
-}
-
 static const struct lock_manager_operations lease_manager_ops = {
 	.fl_break = lease_break_callback,
 	.fl_release_private = lease_release_private_callback,
-	.fl_mylease = lease_mylease_callback,
 	.fl_change = lease_modify,
 };
 
@@ -1405,7 +1399,7 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp)
 	for (before = &inode->i_flock;
 			((fl = *before) != NULL) && IS_LEASE(fl);
 			before = &fl->fl_next) {
-		if (lease->fl_lmops->fl_mylease(fl, lease))
+		if (fl->fl_file == lease->fl_file)
 			my_before = before;
 		else if (fl->fl_type == (F_INPROGRESS | F_UNLCK))
 			/*
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index cbe1b81..87d4c48 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2295,24 +2295,6 @@ void nfsd_break_deleg_cb(struct file_lock *fl)
 	nfsd4_cb_recall(dp);
 }
 
-/*
- * Called from setlease() with lock_flocks() held
- */
-static
-int nfsd_same_client_deleg_cb(struct file_lock *onlist, struct file_lock *try)
-{
-	struct nfs4_delegation *onlistd =
-		(struct nfs4_delegation *)onlist->fl_owner;
-	struct ...
[ message continues ]
[view original message]
From: Christoph Hellwig
Subject: Re: [PATCH 3/4] locks: eliminate fl_mylease callback
Date: Monday, January 3, 2011 - 11:07 pm

Please also update Documentation/filesystems/Locking for method
removals.

--


[ message continues ]
[view original message]
From: J. Bruce Fields
Subject: Re: [PATCH 3/4] locks: eliminate fl_mylease callback
Date: Tuesday, January 4, 2011 - 11:18 am

Whoops, thanks for the reminder.

Looks like we never added fl_mylease?

That leaves the fl_release_private patch, updated as follows.

--b.

commit 3d801116bb23a1f446627ce1976950c7a126541e
Author: J. Bruce Fields <bfields@redhat.com>
Date:   Sat Oct 30 17:41:26 2010 -0400

    nfsd4: eliminate lease delete callback
    
    nfsd controls the lifetime of the lease, not the lock code, so there's
    no need for this callback on lease destruction.
    
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index b6426f1..075be12 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -327,14 +327,12 @@ fl_release_private:	yes	yes
 prototypes:
 	int (*fl_compare_owner)(struct file_lock *, struct file_lock *);
 	void (*fl_notify)(struct file_lock *);  /* unblock callback */
-	void (*fl_release_private)(struct file_lock *);
 	void (*fl_break)(struct file_lock *); /* break_lease callback */
 
 locking rules:
 			BKL	may block
 fl_compare_owner:	yes	no
 fl_notify:		yes	no
-fl_release_private:	yes	yes
 fl_break:		yes	no
 
 	Currently only NFSD and NLM provide instances of this class. None of the
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b82e368..2e44ad2 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2296,23 +2296,6 @@ void nfsd_break_deleg_cb(struct file_lock *fl)
 }
 
 /*
- * The file_lock is being reapd.
- *
- * Called by locks_free_lock() with lock_flocks() held.
- */
-static
-void nfsd_release_deleg_cb(struct file_lock *fl)
-{
-	struct nfs4_delegation *dp = (struct nfs4_delegation *)fl->fl_owner;
-
-	dprintk("NFSD nfsd_release_deleg_cb: fl %p dp %p dl_count %d\n", fl,dp, atomic_read(&dp->dl_count));
-
-	if (!(fl->fl_flags & FL_LEASE) || !dp)
-		return;
-	dp->dl_flock = NULL;
-}
-
-/*
  * Called from setlease() with lock_flocks() held
  */
 static
@@ -2341,7 +2324,6 @@ int ...
[ message continues ]
[view original message]
From: J. Bruce Fields
Subject: [PATCH 1/4] nfsd4: eliminate lease delete callback
Date: Monday, January 3, 2011 - 8:06 pm

nfsd controls the lifetime of the lease, not the lock code, so there's
no need for this callback on lease destruction.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c |   18 ------------------
 1 files changed, 0 insertions(+), 18 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b82e368..2e44ad2 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2296,23 +2296,6 @@ void nfsd_break_deleg_cb(struct file_lock *fl)
 }
 
 /*
- * The file_lock is being reapd.
- *
- * Called by locks_free_lock() with lock_flocks() held.
- */
-static
-void nfsd_release_deleg_cb(struct file_lock *fl)
-{
-	struct nfs4_delegation *dp = (struct nfs4_delegation *)fl->fl_owner;
-
-	dprintk("NFSD nfsd_release_deleg_cb: fl %p dp %p dl_count %d\n", fl,dp, atomic_read(&dp->dl_count));
-
-	if (!(fl->fl_flags & FL_LEASE) || !dp)
-		return;
-	dp->dl_flock = NULL;
-}
-
-/*
  * Called from setlease() with lock_flocks() held
  */
 static
@@ -2341,7 +2324,6 @@ int nfsd_change_deleg_cb(struct file_lock **onlist, int arg)
 
 static const struct lock_manager_operations nfsd_lease_mng_ops = {
 	.fl_break = nfsd_break_deleg_cb,
-	.fl_release_private = nfsd_release_deleg_cb,
 	.fl_mylease = nfsd_same_client_deleg_cb,
 	.fl_change = nfsd_change_deleg_cb,
 };
-- 
1.7.1

--


[ message continues ]
[view original message]
From: J. Bruce Fields
Subject: [PATCH 4/4] locks: minor setlease cleanup
Date: Monday, January 3, 2011 - 8:06 pm

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/locks.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 5cb6506..feaac63 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1399,7 +1399,7 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp)
 	for (before = &inode->i_flock;
 			((fl = *before) != NULL) && IS_LEASE(fl);
 			before = &fl->fl_next) {
-		if (fl->fl_file == lease->fl_file)
+		if (fl->fl_file == filp)
 			my_before = before;
 		else if (fl->fl_type == (F_INPROGRESS | F_UNLCK))
 			/*
-- 
1.7.1

--


[ message continues ]
Previous thread: Re: [PATCH] Update atime from future. by Steven Whitehouse on Monday, January 3, 2011 - 9:41 am. (1 message)

Next thread: [PATCH v2 0/5] add new ioctls to do metadata readahead in btrfs by Shaohua Li on Monday, January 3, 2011 - 10:40 pm. (2 messages)