On Tue, 19 Aug 2014 23:26:45 +0800
Kinglong Mee <kinglongmee@xxxxxxxxx> wrote:
> v4: same as v3, no change
> v3: Update based on Jeff's comments
> v2: Fix bad using of struct file_lock_operations for handle the owner.
>
> Signed-off-by: Kinglong Mee <kinglongmee@xxxxxxxxx>
> ---
> fs/nfsd/nfs4state.c | 34 ++++++++++++++++++++++++++++++----
> 1 file changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index e087a71..7161111 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4869,9 +4869,31 @@ nfs4_transform_lock_offset(struct file_lock *lock)
> lock->fl_end = OFFSET_MAX;
> }
>
> -/* Hack!: For now, we're defining this just so we can use a pointer to it
> - * as a unique cookie to identify our (NFSv4's) posix locks. */
> +static inline struct nfs4_lockowner *
> +nfs4_get_lockowner(struct nfs4_lockowner *lo)
> +{
> + return lockowner(nfs4_get_stateowner(&lo->lo_owner));
> +}
> +
I'd probably not bother with this inline function. Just open code that
into the callers.
> +static void nfsd4_fl_get_owner(struct file_lock *dst, struct file_lock *src)
> +{
> + struct nfs4_lockowner *lo = (struct nfs4_lockowner *) src->fl_owner;
> + dst->fl_owner = (fl_owner_t) nfs4_get_lockowner(lo);
> +}
> +
> +static void nfsd4_fl_put_owner(struct file_lock *fl)
> +{
> + struct nfs4_lockowner *lo = (struct nfs4_lockowner *) fl->fl_owner;
> +
> + if (lo) {
> + nfs4_put_stateowner(&lo->lo_owner);
> + fl->fl_owner = NULL;
> + }
> +}
> +
> static const struct lock_manager_operations nfsd_posix_mng_ops = {
> + .lm_get_owner = nfsd4_fl_get_owner,
> + .lm_put_owner = nfsd4_fl_put_owner,
> };
>
> static inline void
> @@ -5236,7 +5258,8 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> status = nfserr_openmode;
> goto out;
> }
> - file_lock->fl_owner = (fl_owner_t)lock_sop;
> +
> + file_lock->fl_owner = (fl_owner_t) nfs4_get_lockowner(lock_sop);
> file_lock->fl_pid = current->tgid;
> file_lock->fl_file = filp;
> file_lock->fl_flags = FL_POSIX;
> @@ -5403,6 +5426,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> struct nfs4_ol_stateid *stp;
> struct file *filp = NULL;
> struct file_lock *file_lock = NULL;
> + struct nfs4_lockowner *lock_sop = NULL;
nit: Probably no need to initialize lock_sop to NULL. Even better, I'd
just drop that and change the fl_owner assignment below.
> __be32 status;
> int err;
> struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> @@ -5424,6 +5448,8 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> status = nfserr_lock_range;
> goto put_stateid;
> }
> +
> + lock_sop = lockowner(stp->st_stateowner);
> file_lock = locks_alloc_lock();
> if (!file_lock) {
> dprintk("NFSD: %s: unable to allocate lock!\n", __func__);
> @@ -5432,7 +5458,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> }
>
> file_lock->fl_type = F_UNLCK;
> - file_lock->fl_owner = (fl_owner_t)lockowner(stp->st_stateowner);
> + file_lock->fl_owner = (fl_owner_t) nfs4_get_lockowner(lock_sop);
I'd do this instead and not bother with a nfs4_get_lockowner at all...
file_lock->fl_owner = (fl_owner_t)lockowner(nfs4_get_stateowner(stp->st_stateowner));
> file_lock->fl_pid = current->tgid;
> file_lock->fl_file = filp;
> file_lock->fl_flags = FL_POSIX;
But those are minor nits. This looks fine otherwise.
Bruce, if it's OK by you, I'll just take the whole series once Kinglong
respins. It does touch some nfsd code, but it hopefully shouldn't cause
much in the way of conflicts with anything you have queued for v3.18.
Acked-by: Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
