[PATCH] NFSv4: Clean up nfs4 lock state accounting
Ensure that lock owner structures are not released prematurely.
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 7c6f1d6..ec1a22d 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -128,6 +128,7 @@
struct nfs4_lock_state {
struct list_head ls_locks; /* Other lock stateids */
+ struct nfs4_state * ls_state; /* Pointer to open state */
fl_owner_t ls_owner; /* POSIX lock owner */
#define NFS_LOCK_INITIALIZED 1
int ls_flags;
@@ -153,7 +154,7 @@
unsigned long flags; /* Do we hold any locks? */
struct semaphore lock_sema; /* Serializes file locking operations */
- rwlock_t state_lock; /* Protects the lock_states list */
+ spinlock_t state_lock; /* Protects the lock_states list */
nfs4_stateid stateid;
@@ -225,12 +226,8 @@
extern struct nfs4_state *nfs4_find_state(struct inode *, struct rpc_cred *, mode_t mode);
extern void nfs4_increment_seqid(int status, struct nfs4_state_owner *sp);
extern void nfs4_schedule_state_recovery(struct nfs4_client *);
-extern struct nfs4_lock_state *nfs4_find_lock_state(struct nfs4_state *state, fl_owner_t);
-extern struct nfs4_lock_state *nfs4_get_lock_state(struct nfs4_state *state, fl_owner_t);
-extern void nfs4_put_lock_state(struct nfs4_lock_state *state);
+extern int nfs4_set_lock_state(struct nfs4_state *state, struct file_lock *fl);
extern void nfs4_increment_lock_seqid(int status, struct nfs4_lock_state *ls);
-extern void nfs4_notify_setlk(struct nfs4_state *, struct file_lock *, struct nfs4_lock_state *);
-extern void nfs4_notify_unlck(struct nfs4_state *, struct file_lock *, struct nfs4_lock_state *);
extern void nfs4_copy_stateid(nfs4_stateid *, struct nfs4_state *, fl_owner_t);
extern const nfs4_stateid zero_stateid;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index af80b59..0ddc201 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2626,14 +2626,11 @@
down_read(&clp->cl_sem);
nlo.clientid = clp->cl_clientid;
down(&state->lock_sema);
- lsp = nfs4_find_lock_state(state, request->fl_owner);
- if (lsp)
- nlo.id = lsp->ls_id;
- else {
- spin_lock(&clp->cl_lock);
- nlo.id = nfs4_alloc_lockowner_id(clp);
- spin_unlock(&clp->cl_lock);
- }
+ status = nfs4_set_lock_state(state, request);
+ if (status != 0)
+ goto out;
+ lsp = request->fl_u.nfs4_fl.owner;
+ nlo.id = lsp->ls_id;
arg.u.lockt = &nlo;
status = rpc_call_sync(server->client, &msg, 0);
if (!status) {
@@ -2654,8 +2651,7 @@
request->fl_pid = 0;
status = 0;
}
- if (lsp)
- nfs4_put_lock_state(lsp);
+out:
up(&state->lock_sema);
up_read(&clp->cl_sem);
return status;
@@ -2715,28 +2711,26 @@
};
struct nfs4_lock_state *lsp;
struct nfs_locku_opargs luargs;
- int status = 0;
+ int status;
down_read(&clp->cl_sem);
down(&state->lock_sema);
- lsp = nfs4_find_lock_state(state, request->fl_owner);
- if (!lsp)
+ status = nfs4_set_lock_state(state, request);
+ if (status != 0)
goto out;
+ lsp = request->fl_u.nfs4_fl.owner;
/* We might have lost the locks! */
- if ((lsp->ls_flags & NFS_LOCK_INITIALIZED) != 0) {
- luargs.seqid = lsp->ls_seqid;
- memcpy(&luargs.stateid, &lsp->ls_stateid, sizeof(luargs.stateid));
- arg.u.locku = &luargs;
- status = rpc_call_sync(server->client, &msg, RPC_TASK_NOINTR);
- nfs4_increment_lock_seqid(status, lsp);
- }
+ if ((lsp->ls_flags & NFS_LOCK_INITIALIZED) == 0)
+ goto out;
+ luargs.seqid = lsp->ls_seqid;
+ memcpy(&luargs.stateid, &lsp->ls_stateid, sizeof(luargs.stateid));
+ arg.u.locku = &luargs;
+ status = rpc_call_sync(server->client, &msg, RPC_TASK_NOINTR);
+ nfs4_increment_lock_seqid(status, lsp);
- if (status == 0) {
+ if (status == 0)
memcpy(&lsp->ls_stateid, &res.u.stateid,
sizeof(lsp->ls_stateid));
- nfs4_notify_unlck(state, request, lsp);
- }
- nfs4_put_lock_state(lsp);
out:
up(&state->lock_sema);
if (status == 0)
@@ -2762,7 +2756,7 @@
{
struct inode *inode = state->inode;
struct nfs_server *server = NFS_SERVER(inode);
- struct nfs4_lock_state *lsp;
+ struct nfs4_lock_state *lsp = request->fl_u.nfs4_fl.owner;
struct nfs_lockargs arg = {
.fh = NFS_FH(inode),
.type = nfs4_lck_type(cmd, request),
@@ -2784,9 +2778,6 @@
};
int status;
- lsp = nfs4_get_lock_state(state, request->fl_owner);
- if (lsp == NULL)
- return -ENOMEM;
if (!(lsp->ls_flags & NFS_LOCK_INITIALIZED)) {
struct nfs4_state_owner *owner = state->owner;
struct nfs_open_to_lock otl = {
@@ -2808,27 +2799,26 @@
* seqid mutating errors */
nfs4_increment_seqid(status, owner);
up(&owner->so_sema);
+ if (status == 0) {
+ lsp->ls_flags |= NFS_LOCK_INITIALIZED;
+ lsp->ls_seqid++;
+ }
} else {
struct nfs_exist_lock el = {
.seqid = lsp->ls_seqid,
};
memcpy(&el.stateid, &lsp->ls_stateid, sizeof(el.stateid));
largs.u.exist_lock = ⪙
- largs.new_lock_owner = 0;
arg.u.lock = &largs;
status = rpc_call_sync(server->client, &msg, RPC_TASK_NOINTR);
+ /* increment seqid on success, and * seqid mutating errors*/
+ nfs4_increment_lock_seqid(status, lsp);
}
- /* increment seqid on success, and * seqid mutating errors*/
- nfs4_increment_lock_seqid(status, lsp);
/* save the returned stateid. */
- if (status == 0) {
+ if (status == 0)
memcpy(&lsp->ls_stateid, &res.u.stateid, sizeof(nfs4_stateid));
- lsp->ls_flags |= NFS_LOCK_INITIALIZED;
- if (!reclaim)
- nfs4_notify_setlk(state, request, lsp);
- } else if (status == -NFS4ERR_DENIED)
+ else if (status == -NFS4ERR_DENIED)
status = -EAGAIN;
- nfs4_put_lock_state(lsp);
return status;
}
@@ -2869,7 +2859,9 @@
down_read(&clp->cl_sem);
down(&state->lock_sema);
- status = _nfs4_do_setlk(state, cmd, request, 0);
+ status = nfs4_set_lock_state(state, request);
+ if (status == 0)
+ status = _nfs4_do_setlk(state, cmd, request, 0);
up(&state->lock_sema);
if (status == 0) {
/* Note: we always want to sleep here! */
@@ -2927,7 +2919,6 @@
if (signalled())
break;
} while(status < 0);
-
return status;
}
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 591ad1d..afe587d 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -360,7 +360,7 @@
atomic_set(&state->count, 1);
INIT_LIST_HEAD(&state->lock_states);
init_MUTEX(&state->lock_sema);
- rwlock_init(&state->state_lock);
+ spin_lock_init(&state->state_lock);
return state;
}
@@ -542,16 +542,6 @@
return NULL;
}
-struct nfs4_lock_state *
-nfs4_find_lock_state(struct nfs4_state *state, fl_owner_t fl_owner)
-{
- struct nfs4_lock_state *lsp;
- read_lock(&state->state_lock);
- lsp = __nfs4_find_lock_state(state, fl_owner);
- read_unlock(&state->state_lock);
- return lsp;
-}
-
/*
* Return a compatible lock_state. If no initialized lock_state structure
* exists, return an uninitialized one.
@@ -568,14 +558,13 @@
return NULL;
lsp->ls_flags = 0;
lsp->ls_seqid = 0; /* arbitrary */
- lsp->ls_id = -1;
memset(lsp->ls_stateid.data, 0, sizeof(lsp->ls_stateid.data));
atomic_set(&lsp->ls_count, 1);
lsp->ls_owner = fl_owner;
- INIT_LIST_HEAD(&lsp->ls_locks);
spin_lock(&clp->cl_lock);
lsp->ls_id = nfs4_alloc_lockowner_id(clp);
spin_unlock(&clp->cl_lock);
+ INIT_LIST_HEAD(&lsp->ls_locks);
return lsp;
}
@@ -585,34 +574,103 @@
*
* The caller must be holding state->lock_sema and clp->cl_sem
*/
-struct nfs4_lock_state *nfs4_get_lock_state(struct nfs4_state *state, fl_owner_t owner)
+static struct nfs4_lock_state *nfs4_get_lock_state(struct nfs4_state *state, fl_owner_t owner)
{
- struct nfs4_lock_state * lsp;
+ struct nfs4_lock_state *lsp, *new = NULL;
- lsp = nfs4_find_lock_state(state, owner);
- if (lsp == NULL)
- lsp = nfs4_alloc_lock_state(state, owner);
+ for(;;) {
+ spin_lock(&state->state_lock);
+ lsp = __nfs4_find_lock_state(state, owner);
+ if (lsp != NULL)
+ break;
+ if (new != NULL) {
+ new->ls_state = state;
+ list_add(&new->ls_locks, &state->lock_states);
+ set_bit(LK_STATE_IN_USE, &state->flags);
+ lsp = new;
+ new = NULL;
+ break;
+ }
+ spin_unlock(&state->state_lock);
+ new = nfs4_alloc_lock_state(state, owner);
+ if (new == NULL)
+ return NULL;
+ }
+ spin_unlock(&state->state_lock);
+ kfree(new);
return lsp;
}
/*
+ * Release reference to lock_state, and free it if we see that
+ * it is no longer in use
+ */
+static void nfs4_put_lock_state(struct nfs4_lock_state *lsp)
+{
+ struct nfs4_state *state;
+
+ if (lsp == NULL)
+ return;
+ state = lsp->ls_state;
+ if (!atomic_dec_and_lock(&lsp->ls_count, &state->state_lock))
+ return;
+ list_del(&lsp->ls_locks);
+ if (list_empty(&state->lock_states))
+ clear_bit(LK_STATE_IN_USE, &state->flags);
+ spin_unlock(&state->state_lock);
+ kfree(lsp);
+}
+
+static void nfs4_fl_copy_lock(struct file_lock *dst, struct file_lock *src)
+{
+ struct nfs4_lock_state *lsp = src->fl_u.nfs4_fl.owner;
+
+ dst->fl_u.nfs4_fl.owner = lsp;
+ atomic_inc(&lsp->ls_count);
+}
+
+static void nfs4_fl_release_lock(struct file_lock *fl)
+{
+ nfs4_put_lock_state(fl->fl_u.nfs4_fl.owner);
+}
+
+static struct file_lock_operations nfs4_fl_lock_ops = {
+ .fl_copy_lock = nfs4_fl_copy_lock,
+ .fl_release_private = nfs4_fl_release_lock,
+};
+
+int nfs4_set_lock_state(struct nfs4_state *state, struct file_lock *fl)
+{
+ struct nfs4_lock_state *lsp;
+
+ if (fl->fl_ops != NULL)
+ return 0;
+ lsp = nfs4_get_lock_state(state, fl->fl_owner);
+ if (lsp == NULL)
+ return -ENOMEM;
+ fl->fl_u.nfs4_fl.owner = lsp;
+ fl->fl_ops = &nfs4_fl_lock_ops;
+ return 0;
+}
+
+/*
* Byte-range lock aware utility to initialize the stateid of read/write
* requests.
*/
-void
-nfs4_copy_stateid(nfs4_stateid *dst, struct nfs4_state *state, fl_owner_t fl_owner)
+void nfs4_copy_stateid(nfs4_stateid *dst, struct nfs4_state *state, fl_owner_t fl_owner)
{
- if (test_bit(LK_STATE_IN_USE, &state->flags)) {
- struct nfs4_lock_state *lsp;
+ struct nfs4_lock_state *lsp;
- lsp = nfs4_find_lock_state(state, fl_owner);
- if (lsp) {
- memcpy(dst, &lsp->ls_stateid, sizeof(*dst));
- nfs4_put_lock_state(lsp);
- return;
- }
- }
memcpy(dst, &state->stateid, sizeof(*dst));
+ if (test_bit(LK_STATE_IN_USE, &state->flags) == 0)
+ return;
+
+ spin_lock(&state->state_lock);
+ lsp = __nfs4_find_lock_state(state, fl_owner);
+ if (lsp != NULL && (lsp->ls_flags & NFS_LOCK_INITIALIZED) != 0)
+ memcpy(dst, &lsp->ls_stateid, sizeof(*dst));
+ spin_unlock(&state->state_lock);
+ nfs4_put_lock_state(lsp);
}
/*
@@ -624,84 +682,6 @@
lsp->ls_seqid++;
}
-/*
-* Check to see if the request lock (type FL_UNLK) effects the fl lock.
-*
-* fl and request must have the same posix owner
-*
-* return:
-* 0 -> fl not effected by request
-* 1 -> fl consumed by request
-*/
-
-static int
-nfs4_check_unlock(struct file_lock *fl, struct file_lock *request)
-{
- if (fl->fl_start >= request->fl_start && fl->fl_end <= request->fl_end)
- return 1;
- return 0;
-}
-
-/*
- * Post an initialized lock_state on the state->lock_states list.
- */
-void nfs4_notify_setlk(struct nfs4_state *state, struct file_lock *request, struct nfs4_lock_state *lsp)
-{
- if (!list_empty(&lsp->ls_locks))
- return;
- atomic_inc(&lsp->ls_count);
- write_lock(&state->state_lock);
- list_add(&lsp->ls_locks, &state->lock_states);
- set_bit(LK_STATE_IN_USE, &state->flags);
- write_unlock(&state->state_lock);
-}
-
-/*
- * to decide to 'reap' lock state:
- * 1) search i_flock for file_locks with fl.lock_state = to ls.
- * 2) determine if unlock will consume found lock.
- * if so, reap
- *
- * else, don't reap.
- *
- */
-void
-nfs4_notify_unlck(struct nfs4_state *state, struct file_lock *request, struct nfs4_lock_state *lsp)
-{
- struct inode *inode = state->inode;
- struct file_lock *fl;
-
- for (fl = inode->i_flock; fl != NULL; fl = fl->fl_next) {
- if (!(fl->fl_flags & FL_POSIX))
- continue;
- if (fl->fl_owner != lsp->ls_owner)
- continue;
- /* Exit if we find at least one lock which is not consumed */
- if (nfs4_check_unlock(fl,request) == 0)
- return;
- }
-
- write_lock(&state->state_lock);
- list_del_init(&lsp->ls_locks);
- if (list_empty(&state->lock_states))
- clear_bit(LK_STATE_IN_USE, &state->flags);
- write_unlock(&state->state_lock);
- nfs4_put_lock_state(lsp);
-}
-
-/*
- * Release reference to lock_state, and free it if we see that
- * it is no longer in use
- */
-void
-nfs4_put_lock_state(struct nfs4_lock_state *lsp)
-{
- if (!atomic_dec_and_test(&lsp->ls_count))
- return;
- BUG_ON (!list_empty(&lsp->ls_locks));
- kfree(lsp);
-}
-
/*
* Called with sp->so_sema and clp->cl_sem held.
*
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9b8b696..e5a8db0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -674,6 +674,7 @@
struct lock_manager_operations *fl_lmops; /* Callbacks for lockmanagers */
union {
struct nfs_lock_info nfs_fl;
+ struct nfs4_lock_info nfs4_fl;
} fl_u;
};
diff --git a/include/linux/nfs_fs_i.h b/include/linux/nfs_fs_i.h
index e9a7495..e2c18dab 100644
--- a/include/linux/nfs_fs_i.h
+++ b/include/linux/nfs_fs_i.h
@@ -16,6 +16,11 @@
struct nlm_lockowner *owner;
};
+struct nfs4_lock_state;
+struct nfs4_lock_info {
+ struct nfs4_lock_state *owner;
+};
+
/*
* Lock flag values
*/