From: "J. Bruce Fields" <[email protected]>
The following fix up some problems that can cause crashes or silently
broken lock guarantees in the reexport case.
Not fixed:
- Attempts to reclaim locks after a reboot of the reexport
server will fail. This at least seems like an improvement
over the current situation, which is that they'll succeed even
in cases where they shouldn't. Complete support for reboot
recovery is a bigger job.
- NFSv4.1+ lock nofications don't work. So, clients have to
poll as they do with NFSv4.0, which is suboptimal, but correct
(and an improvement over the current situation, which is a
kernel oops).
So what we have at this point is a suboptimal lock implementation that
doesn't support lock recovery.
Another alternative might be to turn off file locking entirely in the
re-export case. I'd rather take the incremental improvement and fix the
oopses.
Changes since v1:
- Use ENOGRACE instead of returning NFS-specific error from vfs lock
method.
- Take write opens for write locks in the NLM case (as we always
have in the NFSv4 case).
- Don't block NLM threads waiting for blocking locks.
With those changes I'm passing connecthon tests for NFSv3-4.2 reexports
of an NFSv4.0 filesystem.
--b.
J. Bruce Fields (8):
lockd: lockd server-side shouldn't set fl_ops
nlm: minor nlm_lookup_file argument change
nlm: minor refactoring
lockd: update nlm_lookup_file reexport comment
Keep read and write fds with each nlm_file
nfs: don't atempt blocking locks on nfs reexports
lockd: don't attempt blocking locks on nfs reexports
nfs: don't allow reexport reclaims
fs/lockd/svc4proc.c | 6 +-
fs/lockd/svclock.c | 80 +++++++++++++----------
fs/lockd/svcproc.c | 6 +-
fs/lockd/svcsubs.c | 125 +++++++++++++++++++++++++-----------
fs/nfs/export.c | 2 +-
fs/nfs/file.c | 3 +
fs/nfsd/lockd.c | 8 ++-
fs/nfsd/nfs4state.c | 11 +++-
fs/nfsd/nfsproc.c | 1 +
include/linux/errno.h | 1 +
include/linux/exportfs.h | 2 +
include/linux/fs.h | 1 +
include/linux/lockd/bind.h | 3 +-
include/linux/lockd/lockd.h | 13 ++--
14 files changed, 177 insertions(+), 85 deletions(-)
--
2.31.1
From: "J. Bruce Fields" <[email protected]>
Locks have two sets of op arrays, fl_lmops for the lock manager (lockd
or nfsd), fl_ops for the filesystem. The server-side lockd code has
been setting its own fl_ops, which leads to confusion (and crashes) in
the reexport case, where the filesystem expects to be the only one
setting fl_ops.
And there's no reason for it that I can see-the lm_get/put_owner ops do
the same job.
Reported-by: Daire Byrne <[email protected]>
Tested-by: Daire Byrne <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/lockd/svclock.c | 30 ++++++++++++------------------
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index 61d3cc2283dc..1781fc5e9091 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -395,28 +395,10 @@ nlmsvc_release_lockowner(struct nlm_lock *lock)
nlmsvc_put_lockowner(lock->fl.fl_owner);
}
-static void nlmsvc_locks_copy_lock(struct file_lock *new, struct file_lock *fl)
-{
- struct nlm_lockowner *nlm_lo = (struct nlm_lockowner *)fl->fl_owner;
- new->fl_owner = nlmsvc_get_lockowner(nlm_lo);
-}
-
-static void nlmsvc_locks_release_private(struct file_lock *fl)
-{
- nlmsvc_put_lockowner((struct nlm_lockowner *)fl->fl_owner);
-}
-
-static const struct file_lock_operations nlmsvc_lock_ops = {
- .fl_copy_lock = nlmsvc_locks_copy_lock,
- .fl_release_private = nlmsvc_locks_release_private,
-};
-
void nlmsvc_locks_init_private(struct file_lock *fl, struct nlm_host *host,
pid_t pid)
{
fl->fl_owner = nlmsvc_find_lockowner(host, pid);
- if (fl->fl_owner != NULL)
- fl->fl_ops = &nlmsvc_lock_ops;
}
/*
@@ -788,9 +770,21 @@ nlmsvc_notify_blocked(struct file_lock *fl)
printk(KERN_WARNING "lockd: notification for unknown block!\n");
}
+static fl_owner_t nlmsvc_get_owner(fl_owner_t owner)
+{
+ return nlmsvc_get_lockowner(owner);
+}
+
+static void nlmsvc_put_owner(fl_owner_t owner)
+{
+ nlmsvc_put_lockowner(owner);
+}
+
const struct lock_manager_operations nlmsvc_lock_operations = {
.lm_notify = nlmsvc_notify_blocked,
.lm_grant = nlmsvc_grant_deferred,
+ .lm_get_owner = nlmsvc_get_owner,
+ .lm_put_owner = nlmsvc_put_owner,
};
/*
--
2.31.1
From: "J. Bruce Fields" <[email protected]>
Update comment to reflect that we *do* allow reexport, whether it's a
good idea or not....
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/lockd/svcsubs.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
index 2558598610a9..f43d89e89c45 100644
--- a/fs/lockd/svcsubs.c
+++ b/fs/lockd/svcsubs.c
@@ -111,8 +111,9 @@ nlm_lookup_file(struct svc_rqst *rqstp, struct nlm_file **result,
INIT_HLIST_NODE(&file->f_list);
INIT_LIST_HEAD(&file->f_blocks);
- /* Open the file. Note that this must not sleep for too long, else
- * we would lock up lockd:-) So no NFS re-exports, folks.
+ /*
+ * Open the file. Note that if we're reexporting, for example,
+ * this could block the lockd thread for a while.
*
* We have to make sure we have the right credential to open
* the file.
--
2.31.1