* [PATCH 1/4] nfsd4: eliminate lease delete callback
2011-01-04 3:06 remove lease callbacks J. Bruce Fields
@ 2011-01-04 3:06 ` J. Bruce Fields
2011-01-04 3:06 ` [PATCH 2/4] nfsd4: use a single struct file for delegations J. Bruce Fields
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: J. Bruce Fields @ 2011-01-04 3:06 UTC (permalink / raw)
To: linux-nfs; +Cc: linux-fsdevel, J. Bruce Fields
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/4] nfsd4: use a single struct file for delegations
2011-01-04 3:06 remove lease callbacks J. Bruce Fields
2011-01-04 3:06 ` [PATCH 1/4] nfsd4: eliminate lease delete callback J. Bruce Fields
@ 2011-01-04 3:06 ` J. Bruce Fields
2011-01-04 3:06 ` [PATCH 3/4] locks: eliminate fl_mylease callback J. Bruce Fields
2011-01-04 3:06 ` [PATCH 4/4] locks: minor setlease cleanup J. Bruce Fields
3 siblings, 0 replies; 10+ messages in thread
From: J. Bruce Fields @ 2011-01-04 3:06 UTC (permalink / raw)
To: linux-nfs; +Cc: linux-fsdevel, J. Bruce Fields
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 nfs4_file *dl_file;
+ struct file *dl_vfs_file;
struct file_lock *dl_flock;
u32 dl_type;
time_t dl_time;
--
1.7.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/4] locks: eliminate fl_mylease callback
2011-01-04 3:06 remove lease callbacks J. Bruce Fields
2011-01-04 3:06 ` [PATCH 1/4] nfsd4: eliminate lease delete callback J. Bruce Fields
2011-01-04 3:06 ` [PATCH 2/4] nfsd4: use a single struct file for delegations J. Bruce Fields
@ 2011-01-04 3:06 ` J. Bruce Fields
2011-01-04 6:07 ` Christoph Hellwig
2011-01-04 3:06 ` [PATCH 4/4] locks: minor setlease cleanup J. Bruce Fields
3 siblings, 1 reply; 10+ messages in thread
From: J. Bruce Fields @ 2011-01-04 3:06 UTC (permalink / raw)
To: linux-nfs; +Cc: linux-fsdevel, J. Bruce Fields
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 nfs4_delegation *tryd =
- (struct nfs4_delegation *)try->fl_owner;
-
- if (onlist->fl_lmops != try->fl_lmops)
- return 0;
-
- return onlistd->dl_client == tryd->dl_client;
-}
-
-
static
int nfsd_change_deleg_cb(struct file_lock **onlist, int arg)
{
@@ -2324,7 +2306,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_mylease = nfsd_same_client_deleg_cb,
.fl_change = nfsd_change_deleg_cb,
};
@@ -2630,7 +2611,7 @@ nfs4_open_delegation(struct svc_fh *fh, struct nfsd4_open *open, struct nfs4_sta
dp->dl_flock = fl;
/* vfs_setlease checks to see if delegation should be handed out.
- * the lock_manager callbacks fl_mylease and fl_change are used
+ * the lock_manager callback fl_change is used
*/
if ((status = vfs_setlease(fl->fl_file, fl->fl_type, &fl))) {
dprintk("NFSD: setlease failed [%d], no delegation\n", status);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 090f0ea..73ce69f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1059,7 +1059,6 @@ struct lock_manager_operations {
int (*fl_grant)(struct file_lock *, struct file_lock *, int);
void (*fl_release_private)(struct file_lock *);
void (*fl_break)(struct file_lock *);
- int (*fl_mylease)(struct file_lock *, struct file_lock *);
int (*fl_change)(struct file_lock **, int);
};
--
1.7.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/4] locks: eliminate fl_mylease callback
2011-01-04 3:06 ` [PATCH 3/4] locks: eliminate fl_mylease callback J. Bruce Fields
@ 2011-01-04 6:07 ` Christoph Hellwig
2011-01-04 18:18 ` J. Bruce Fields
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2011-01-04 6:07 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: linux-nfs, linux-fsdevel
On Mon, Jan 03, 2011 at 10:06:58PM -0500, J. Bruce Fields wrote:
> 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.
Please also update Documentation/filesystems/Locking for method
removals.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] locks: eliminate fl_mylease callback
2011-01-04 6:07 ` Christoph Hellwig
@ 2011-01-04 18:18 ` J. Bruce Fields
2011-01-07 13:40 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: J. Bruce Fields @ 2011-01-04 18:18 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: J. Bruce Fields, linux-nfs, linux-fsdevel
On Tue, Jan 04, 2011 at 01:07:23AM -0500, Christoph Hellwig wrote:
> On Mon, Jan 03, 2011 at 10:06:58PM -0500, J. Bruce Fields wrote:
> > 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.
>
> Please also update Documentation/filesystems/Locking for method
> removals.
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 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,
};
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/4] locks: eliminate fl_mylease callback
2011-01-04 18:18 ` J. Bruce Fields
@ 2011-01-07 13:40 ` Christoph Hellwig
2011-01-09 19:55 ` J. Bruce Fields
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2011-01-07 13:40 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: J. Bruce Fields, linux-nfs, linux-fsdevel
On Tue, Jan 04, 2011 at 01:18:16PM -0500, J. Bruce Fields wrote:
> Whoops, thanks for the reminder.
>
> Looks like we never added fl_mylease?
I actually addded it recently, but it seems like you're based on an
older tree.
> That leaves the fl_release_private patch, updated as follows.
I don't think you removed all instances of it yet. It is still
implemented by lease_manager_ops in fs/locks.c
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] locks: eliminate fl_mylease callback
2011-01-07 13:40 ` Christoph Hellwig
@ 2011-01-09 19:55 ` J. Bruce Fields
2011-01-09 20:08 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: J. Bruce Fields @ 2011-01-09 19:55 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: J. Bruce Fields, linux-nfs, linux-fsdevel
On Fri, Jan 07, 2011 at 08:40:15AM -0500, Christoph Hellwig wrote:
> On Tue, Jan 04, 2011 at 01:18:16PM -0500, J. Bruce Fields wrote:
> > Whoops, thanks for the reminder.
> >
> > Looks like we never added fl_mylease?
>
> I actually addded it recently, but it seems like you're based on an
> older tree.
Whoops, I'll probably resolve that before submitting a pull request....
> > That leaves the fl_release_private patch, updated as follows.
>
> I don't think you removed all instances of it yet. It is still
> implemented by lease_manager_ops in fs/locks.c
Argh, yes, I'd forgotten about that. The original patch is still right,
so we could just back out the documentation patch, or maybe there's a
better alternative for the remaining user.
--b.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] locks: eliminate fl_mylease callback
2011-01-09 19:55 ` J. Bruce Fields
@ 2011-01-09 20:08 ` Christoph Hellwig
0 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2011-01-09 20:08 UTC (permalink / raw)
To: J. Bruce Fields
Cc: Christoph Hellwig, J. Bruce Fields, linux-nfs, linux-fsdevel
On Sun, Jan 09, 2011 at 02:55:09PM -0500, J. Bruce Fields wrote:
> > I don't think you removed all instances of it yet. It is still
> > implemented by lease_manager_ops in fs/locks.c
>
> Argh, yes, I'd forgotten about that. The original patch is still right,
> so we could just back out the documentation patch, or maybe there's a
> better alternative for the remaining user.
Given that leases are only releases in a few controlled places the
code could probably be moved into those. But it's probably to risky to
try that so late in the merge window.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] locks: minor setlease cleanup
2011-01-04 3:06 remove lease callbacks J. Bruce Fields
` (2 preceding siblings ...)
2011-01-04 3:06 ` [PATCH 3/4] locks: eliminate fl_mylease callback J. Bruce Fields
@ 2011-01-04 3:06 ` J. Bruce Fields
3 siblings, 0 replies; 10+ messages in thread
From: J. Bruce Fields @ 2011-01-04 3:06 UTC (permalink / raw)
To: linux-nfs; +Cc: linux-fsdevel, J. Bruce Fields
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
^ permalink raw reply related [flat|nested] 10+ messages in thread