diff mbox

NFSD: Don't give out read delegations on exclusive creates

Message ID 1368643909-8059-1-git-send-email-steved@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Steve Dickson May 15, 2013, 6:51 p.m. UTC
When an exclusive create is done with the mode bits
set (aka open(testfile, O_CREAT | O_EXCL, 0777)) this
causes a OPEN op followed by a SETATTR op. When a
read delegation is given in the OPEN, it causes
the SETATTR to delay with EAGAIN until the
delegation is recalled.

This patch caused exclusive creates to give out
a write delegation (which turn into no delegation)
which allows the SETATTR seamlessly succeed.

Signed-off-by: Steve Dickson <steved@redhat.com>
---
 fs/nfsd/nfs4state.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Steve Dickson May 21, 2013, 6:43 p.m. UTC | #1
Hey Bruce,

Ping... ;-) I just pull from your for-3.11-incoming branch
and notice it was not there... 

This  really does help with  exclusive creates 
not being delayed... So you might want to consider it... 

steved.

On 15/05/13 14:51, Steve Dickson wrote:
> When an exclusive create is done with the mode bits
> set (aka open(testfile, O_CREAT | O_EXCL, 0777)) this
> causes a OPEN op followed by a SETATTR op. When a
> read delegation is given in the OPEN, it causes
> the SETATTR to delay with EAGAIN until the
> delegation is recalled.
> 
> This patch caused exclusive creates to give out
> a write delegation (which turn into no delegation)
> which allows the SETATTR seamlessly succeed.
> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
> ---
>  fs/nfsd/nfs4state.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 316ec84..6b45d0e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -3035,8 +3035,16 @@ nfs4_open_delegation(struct net *net, struct svc_fh *fh,
>  				goto out;
>  			if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE)
>  				flag = NFS4_OPEN_DELEGATE_WRITE;
> -			else
> -				flag = NFS4_OPEN_DELEGATE_READ;
> +			else {
> +				switch(open->op_createmode) {
> +				case NFS4_CREATE_EXCLUSIVE:
> +				case NFS4_CREATE_EXCLUSIVE4_1:
> +					flag = NFS4_OPEN_DELEGATE_WRITE;
> +					break;
> +				default:
> +					flag = NFS4_OPEN_DELEGATE_READ;
> +				}
> +			}
>  			break;
>  		default:
>  			goto out;
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields May 21, 2013, 6:52 p.m. UTC | #2
On Tue, May 21, 2013 at 02:43:16PM -0400, Steve Dickson wrote:
> Hey Bruce,
> 
> Ping... ;-) I just pull from your for-3.11-incoming branch
> and notice it was not there... 

Whoops, this had gotten buried in my inbox somehow.  I'll take a look,
thanks for the reminder.

--b.

> 
> This  really does help with  exclusive creates 
> not being delayed... So you might want to consider it... 
> 
> steved.
> 
> On 15/05/13 14:51, Steve Dickson wrote:
> > When an exclusive create is done with the mode bits
> > set (aka open(testfile, O_CREAT | O_EXCL, 0777)) this
> > causes a OPEN op followed by a SETATTR op. When a
> > read delegation is given in the OPEN, it causes
> > the SETATTR to delay with EAGAIN until the
> > delegation is recalled.
> > 
> > This patch caused exclusive creates to give out
> > a write delegation (which turn into no delegation)
> > which allows the SETATTR seamlessly succeed.
> > 
> > Signed-off-by: Steve Dickson <steved@redhat.com>
> > ---
> >  fs/nfsd/nfs4state.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 316ec84..6b45d0e 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -3035,8 +3035,16 @@ nfs4_open_delegation(struct net *net, struct svc_fh *fh,
> >  				goto out;
> >  			if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE)
> >  				flag = NFS4_OPEN_DELEGATE_WRITE;
> > -			else
> > -				flag = NFS4_OPEN_DELEGATE_READ;
> > +			else {
> > +				switch(open->op_createmode) {
> > +				case NFS4_CREATE_EXCLUSIVE:
> > +				case NFS4_CREATE_EXCLUSIVE4_1:
> > +					flag = NFS4_OPEN_DELEGATE_WRITE;
> > +					break;
> > +				default:
> > +					flag = NFS4_OPEN_DELEGATE_READ;
> > +				}
> > +			}
> >  			break;
> >  		default:
> >  			goto out;
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields May 21, 2013, 7:23 p.m. UTC | #3
On Wed, May 15, 2013 at 02:51:49PM -0400, Steve Dickson wrote:
> When an exclusive create is done with the mode bits
> set (aka open(testfile, O_CREAT | O_EXCL, 0777)) this

so implicitly O_RDONLY.  Is that common?  Maybe so, OK.

> causes a OPEN op followed by a SETATTR op. When a
> read delegation is given in the OPEN, it causes
> the SETATTR to delay with EAGAIN until the
> delegation is recalled.
> 
> This patch caused exclusive creates to give out
> a write delegation (which turn into no delegation)
> which allows the SETATTR seamlessly succeed.

OK.  May as well make it apply to all creates, though, I think?
Any create flag seems like a sign the file's likely to be modified soon,
hence isn't a good candidate for a read delegation.

I find it a little confusing the way we set this WRITE flag and then
depend on later code to turn that into no delegation, maybe we should
fix that.

--b.

> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
> ---
>  fs/nfsd/nfs4state.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 316ec84..6b45d0e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -3035,8 +3035,16 @@ nfs4_open_delegation(struct net *net, struct svc_fh *fh,
>  				goto out;
>  			if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE)
>  				flag = NFS4_OPEN_DELEGATE_WRITE;
> -			else
> -				flag = NFS4_OPEN_DELEGATE_READ;
> +			else {
> +				switch(open->op_createmode) {
> +				case NFS4_CREATE_EXCLUSIVE:
> +				case NFS4_CREATE_EXCLUSIVE4_1:
> +					flag = NFS4_OPEN_DELEGATE_WRITE;
> +					break;
> +				default:
> +					flag = NFS4_OPEN_DELEGATE_READ;
> +				}
> +			}
>  			break;
>  		default:
>  			goto out;
> -- 
> 1.8.1.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 316ec84..6b45d0e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3035,8 +3035,16 @@  nfs4_open_delegation(struct net *net, struct svc_fh *fh,
 				goto out;
 			if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE)
 				flag = NFS4_OPEN_DELEGATE_WRITE;
-			else
-				flag = NFS4_OPEN_DELEGATE_READ;
+			else {
+				switch(open->op_createmode) {
+				case NFS4_CREATE_EXCLUSIVE:
+				case NFS4_CREATE_EXCLUSIVE4_1:
+					flag = NFS4_OPEN_DELEGATE_WRITE;
+					break;
+				default:
+					flag = NFS4_OPEN_DELEGATE_READ;
+				}
+			}
 			break;
 		default:
 			goto out;