On Thu, May 15, 2008 at 09:50:20PM +0200, Miklos Szeredi wrote: > From: Miklos Szeredi <mszeredi@xxxxxxx> > > 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@xxxxxxx> > CC: Trond Myklebust <trond.myklebust@xxxxxxxxxx> > CC: "J. Bruce Fields" <bfields@xxxxxxxxxxxx> > --- > 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-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html