bfields at fieldses
May 20, 2008, 11:47 AM
Post #2 of 2
(74 views)
Permalink
|
|
Re: [patch 1/9] lockd: dont return EAGAIN for a permanent error
[In reply to]
|
|
On Thu, May 15, 2008 at 09:50:20PM +0200, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi[at]suse.cz>
>
> Fix nlm_fopen() to return NLM_FAILED (or NLM_LCK_DENIED_NOLOCKS)
> instead of NLM_LCK_DENIED. The latter means the lock request failed
> because of a conflicting lock (i.e. a temporary error), which is wrong
> in this case.
>
> Also fix the client to return ENOLCK instead of EAGAIN if a blocking
> lock request returns with NLM_LOCK_DENIED.
>
> Signed-off-by: Miklos Szeredi <mszeredi[at]suse.cz>
> CC: Trond Myklebust <trond.myklebust[at]fys.uio.no>
> CC: "J. Bruce Fields" <bfields[at]fieldses.org>
> ---
> fs/lockd/clntproc.c | 10 +++++++++-
> fs/nfsd/lockd.c | 13 +++++++++----
> 2 files changed, 18 insertions(+), 5 deletions(-)
>
> Index: linux-2.6/fs/lockd/clntproc.c
> ===================================================================
> --- linux-2.6.orig/fs/lockd/clntproc.c 2008-05-15 17:54:30.000000000 +0200
> +++ linux-2.6/fs/lockd/clntproc.c 2008-05-15 17:59:41.000000000 +0200
> @@ -580,7 +580,15 @@ again:
> }
> if (status < 0)
> goto out_unlock;
> - status = nlm_stat_to_errno(resp->status);
> + /*
> + * EAGAIN doesn't make sense for sleeping locks, and in some
> + * cases NLM_LCK_DENIED is returned for a permanent error. So
Just for the sake of future readers, I might go ahead and give
specifics: "and older versions of the linux server sometimes returned
NLM_LCK_DENIED for permanent errors", or something.
> + * turn it into an ENOLCK.
> + */
> + if (resp->status == nlm_lck_denied && (fl_flags & FL_SLEEP))
> + status = -ENOLCK;
> + else
> + status = nlm_stat_to_errno(resp->status);
Might be a bit clearer (or at least make the comment and code more
obviously agree) to do:
status = nlm_stat_to_errno(resp->status);
if (status == -EAGAIN && (fl_flags & FL_SLEEP))
status = -ENOLCK;
This might make more sense as separate client and server-side patches.
Seems reasonable to me otherwise.
--b.
> out_unblock:
> nlmclnt_finish_block(block);
> out:
> Index: linux-2.6/fs/nfsd/lockd.c
> ===================================================================
> --- linux-2.6.orig/fs/nfsd/lockd.c 2008-05-15 17:54:30.000000000 +0200
> +++ linux-2.6/fs/nfsd/lockd.c 2008-05-15 17:57:45.000000000 +0200
> @@ -19,6 +19,13 @@
>
> #define NFSDDBG_FACILITY NFSDDBG_LOCKD
>
> +#ifdef CONFIG_LOCKD_V4
> +#define nlm_stale_fh nlm4_stale_fh
> +#define nlm_failed nlm4_failed
> +#else
> +#define nlm_stale_fh nlm_lck_denied_nolocks
> +#define nlm_failed nlm_lck_denied_nolocks
> +#endif
> /*
> * Note: we hold the dentry use count while the file is open.
> */
> @@ -47,12 +54,10 @@ nlm_fopen(struct svc_rqst *rqstp, struct
> return 0;
> case nfserr_dropit:
> return nlm_drop_reply;
> -#ifdef CONFIG_LOCKD_V4
> case nfserr_stale:
> - return nlm4_stale_fh;
> -#endif
> + return nlm_stale_fh;
> default:
> - return nlm_lck_denied;
> + return nlm_failed;
> }
> }
>
>
> --
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
|
