Linux NFS development
 help / color / mirror / Atom feed
* [PATCH] nfs41: change PNFS_LAYOUTRET_ON_SETATTR to only return on chown/truncate
@ 2014-09-11 15:02 Peng Tao
  2014-09-11 20:11 ` Christoph Hellwig
  0 siblings, 1 reply; 5+ messages in thread
From: Peng Tao @ 2014-09-11 15:02 UTC (permalink / raw)
  To: linux-nfs; +Cc: Peng Tao, Christoph Hellwig, Boaz Harrosh

Both blocks layout and objects layout want to use it to avoid CB_LAYOUTRECALL
but that should only happen if client is doing chown or truncate. So change
PNFS_LAYOUTRET_ON_SETATTR to follow the logic and not to send layoutreturn
unnecessarily.

Cc: Christoph Hellwig <hch@lst.de>
Cc: Boaz Harrosh <boaz@plexistor.com>
Signed-off-by: Peng Tao <tao.peng@primarydata.com>
---
 fs/nfs/nfs4proc.c | 3 ++-
 fs/nfs/pnfs.h     | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7dd8aca..e55bd0f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3213,7 +3213,8 @@ nfs4_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr,
 	struct nfs4_label *label = NULL;
 	int status;
 
-	if (pnfs_ld_layoutret_on_setattr(inode))
+	if (pnfs_ld_layoutret_on_setattr(inode) &&
+	    sattr->ia_valid & (ATTR_UID|ATTR_GID|ATTR_SIZE))
 		pnfs_commit_and_return_layout(inode);
 
 	nfs_fattr_init(fattr);
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index aca3dff..1765ea8 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -68,7 +68,8 @@ enum {
 };
 
 enum layoutdriver_policy_flags {
-	/* Should the pNFS client commit and return the layout upon a setattr */
+	/* Should the pNFS client commit and return the layout upon a setattr
+	 * that changes uid/gid/size. */
 	PNFS_LAYOUTRET_ON_SETATTR	= 1 << 0,
 	PNFS_LAYOUTRET_ON_ERROR		= 1 << 1,
 };
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] nfs41: change PNFS_LAYOUTRET_ON_SETATTR to only return on chown/truncate
  2014-09-11 15:02 [PATCH] nfs41: change PNFS_LAYOUTRET_ON_SETATTR to only return on chown/truncate Peng Tao
@ 2014-09-11 20:11 ` Christoph Hellwig
  2014-09-12  2:23   ` Peng Tao
  0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2014-09-11 20:11 UTC (permalink / raw)
  To: Peng Tao; +Cc: linux-nfs, Christoph Hellwig, Boaz Harrosh

On Thu, Sep 11, 2014 at 11:02:32PM +0800, Peng Tao wrote:
> Both blocks layout and objects layout want to use it to avoid CB_LAYOUTRECALL
> but that should only happen if client is doing chown or truncate. So change
> PNFS_LAYOUTRET_ON_SETATTR to follow the logic and not to send layoutreturn
> unnecessarily.

We should defintively reduce the amount of returns, but why do we even
bother and chown and a truncate to a larger size?


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] nfs41: change PNFS_LAYOUTRET_ON_SETATTR to only return on chown/truncate
  2014-09-11 20:11 ` Christoph Hellwig
@ 2014-09-12  2:23   ` Peng Tao
  0 siblings, 0 replies; 5+ messages in thread
From: Peng Tao @ 2014-09-12  2:23 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Linux NFS Mailing List, Christoph Hellwig, Boaz Harrosh

On Fri, Sep 12, 2014 at 4:11 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Thu, Sep 11, 2014 at 11:02:32PM +0800, Peng Tao wrote:
>> Both blocks layout and objects layout want to use it to avoid CB_LAYOUTRECALL
>> but that should only happen if client is doing chown or truncate. So change
>> PNFS_LAYOUTRET_ON_SETATTR to follow the logic and not to send layoutreturn
>> unnecessarily.
>
> We should defintively reduce the amount of returns, but why do we even
> bother and chown and a truncate to a larger size?
hmm, I agree. Let's only layoutreturn on truncate to a smaller size
and let server decide if it needs CB_LAYOUTRECALL in chown.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] nfs41: change PNFS_LAYOUTRET_ON_SETATTR to only return on chown/truncate
@ 2014-09-12  3:01 Peng Tao
  2014-09-12  3:03 ` Peng Tao
  0 siblings, 1 reply; 5+ messages in thread
From: Peng Tao @ 2014-09-12  3:01 UTC (permalink / raw)
  To: linux-nfs; +Cc: Peng Tao, Christoph Hellwig, Boaz Harrosh

Both blocks layout and objects layout want to use it to avoid CB_LAYOUTRECALL
but that should only happen if client is doing chown or truncate. So change
PNFS_LAYOUTRET_ON_SETATTR to follow the logic and not to send layoutreturn
unnecessarily.

Cc: Christoph Hellwig <hch@lst.de>
Cc: Boaz Harrosh <boaz@plexistor.com>
Signed-off-by: Peng Tao <tao.peng@primarydata.com>
---
 fs/nfs/nfs4proc.c | 3 ++-
 fs/nfs/pnfs.h     | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7dd8aca..e55bd0f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3213,7 +3213,8 @@ nfs4_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr,
 	struct nfs4_label *label = NULL;
 	int status;
 
-	if (pnfs_ld_layoutret_on_setattr(inode))
+	if (pnfs_ld_layoutret_on_setattr(inode) &&
+	    sattr->ia_valid & (ATTR_UID|ATTR_GID|ATTR_SIZE))
 		pnfs_commit_and_return_layout(inode);
 
 	nfs_fattr_init(fattr);
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index aca3dff..1765ea8 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -68,7 +68,8 @@ enum {
 };
 
 enum layoutdriver_policy_flags {
-	/* Should the pNFS client commit and return the layout upon a setattr */
+	/* Should the pNFS client commit and return the layout upon a setattr
+	 * that changes uid/gid/size. */
 	PNFS_LAYOUTRET_ON_SETATTR	= 1 << 0,
 	PNFS_LAYOUTRET_ON_ERROR		= 1 << 1,
 };
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] nfs41: change PNFS_LAYOUTRET_ON_SETATTR to only return on chown/truncate
  2014-09-12  3:01 Peng Tao
@ 2014-09-12  3:03 ` Peng Tao
  0 siblings, 0 replies; 5+ messages in thread
From: Peng Tao @ 2014-09-12  3:03 UTC (permalink / raw)
  To: Linux NFS Mailing List; +Cc: Peng Tao, Christoph Hellwig, Boaz Harrosh

sorry, sent out wrong patch. Please ignore this.

On Fri, Sep 12, 2014 at 11:01 AM, Peng Tao <tao.peng@primarydata.com> wrote:
> Both blocks layout and objects layout want to use it to avoid CB_LAYOUTRECALL
> but that should only happen if client is doing chown or truncate. So change
> PNFS_LAYOUTRET_ON_SETATTR to follow the logic and not to send layoutreturn
> unnecessarily.
>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Boaz Harrosh <boaz@plexistor.com>
> Signed-off-by: Peng Tao <tao.peng@primarydata.com>
> ---
>  fs/nfs/nfs4proc.c | 3 ++-
>  fs/nfs/pnfs.h     | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 7dd8aca..e55bd0f 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3213,7 +3213,8 @@ nfs4_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr,
>         struct nfs4_label *label = NULL;
>         int status;
>
> -       if (pnfs_ld_layoutret_on_setattr(inode))
> +       if (pnfs_ld_layoutret_on_setattr(inode) &&
> +           sattr->ia_valid & (ATTR_UID|ATTR_GID|ATTR_SIZE))
>                 pnfs_commit_and_return_layout(inode);
>
>         nfs_fattr_init(fattr);
> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> index aca3dff..1765ea8 100644
> --- a/fs/nfs/pnfs.h
> +++ b/fs/nfs/pnfs.h
> @@ -68,7 +68,8 @@ enum {
>  };
>
>  enum layoutdriver_policy_flags {
> -       /* Should the pNFS client commit and return the layout upon a setattr */
> +       /* Should the pNFS client commit and return the layout upon a setattr
> +        * that changes uid/gid/size. */
>         PNFS_LAYOUTRET_ON_SETATTR       = 1 << 0,
>         PNFS_LAYOUTRET_ON_ERROR         = 1 << 1,
>  };
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-09-12  3:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-11 15:02 [PATCH] nfs41: change PNFS_LAYOUTRET_ON_SETATTR to only return on chown/truncate Peng Tao
2014-09-11 20:11 ` Christoph Hellwig
2014-09-12  2:23   ` Peng Tao
  -- strict thread matches above, loose matches on Subject: below --
2014-09-12  3:01 Peng Tao
2014-09-12  3:03 ` Peng Tao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox