| Message ID | 1410491050-12858-1-git-send-email-tao.peng@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 09/12/2014 06:04 AM, 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 truncation to a smaller size. > For other cases, we let server decide if it wants to recall client's layouts. > 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> Peng, Christoph Hi This is in violation of the pnf-objects draft. We are obliged on chown to return layouts because of how the CAPs work, they have an embedded CAPs version which might increment when chown is changed, and also the credential keys. Which means that using the old layout will return an E_ACCESS from the OSD. But.. I will agree with this patch. The above is true in an OSD protocol higher then NO_SEC. But none of the current implementation support anything other then NO_SEC. Second also with none-NO_SEC once a client does a chown the server needs to RECALL all other clients, so as Peng says above the Server should/can also recall from us. But some good sole needs to make an errata at the RFC draft so to explain. So: ACK-by: Boaz Harrosh <bharrosh@panasas.com> > Signed-off-by: Peng Tao <tao.peng@primarydata.com> > --- > fs/nfs/nfs4proc.c | 4 +++- > fs/nfs/pnfs.h | 3 ++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index 7dd8aca..63a5b80 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -3213,7 +3213,9 @@ 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_SIZE && > + sattr->ia_size < i_size_read(inode)) > pnfs_commit_and_return_layout(inode); > > nfs_fattr_init(fattr); > diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h > index aca3dff..04a3646 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 truncate to > + * a smaller size */ > PNFS_LAYOUTRET_ON_SETATTR = 1 << 0, > PNFS_LAYOUTRET_ON_ERROR = 1 << 1, > }; > -- 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
On Sun, Sep 14, 2014 at 01:49:34PM +0300, Boaz Harrosh wrote: > This is in violation of the pnf-objects draft. Ithought that's an RFC by now? :) > We are obliged > on chown to return layouts because of how the CAPs work, they > have an embedded CAPs version which might increment when chown > is changed, and also the credential keys. Which means that using > the old layout will return an E_ACCESS from the OSD. This is a good enough argument to just split the flags into one for chown and one for truncate. Can you confirm that any other setattr but a truncate to a smaller size or chown is fine with your interpretation of RFC5664? > But.. > > I will agree with this patch. The above is true in an OSD > protocol higher then NO_SEC. But none of the current implementation > support anything other then NO_SEC. > > Second also with none-NO_SEC once a client does a chown the server > needs to RECALL all other clients, so as Peng says above the Server > should/can also recall from us. > > But some good sole needs to make an errata at the RFC draft so to > explain. I'd rather not violate the existing RFC unless we have a good reason for it and an errata out. Having a flag to return on chown for the object layout seems like the easier way to go forward. -- 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
On 09/14/2014 07:43 PM, Christoph Hellwig wrote: > On Sun, Sep 14, 2014 at 01:49:34PM +0300, Boaz Harrosh wrote: >> This is in violation of the pnf-objects draft. > Ithought that's an RFC by now? :) > >> We are obliged >> on chown to return layouts because of how the CAPs work, they >> have an embedded CAPs version which might increment when chown >> is changed, and also the credential keys. Which means that using >> the old layout will return an E_ACCESS from the OSD. > This is a good enough argument to just split the flags into one > for chown and one for truncate. Can you confirm that any other > setattr but a truncate to a smaller size or chown is fine with > your interpretation of RFC5664? > >> But.. >> >> I will agree with this patch. The above is true in an OSD >> protocol higher then NO_SEC. But none of the current implementation >> support anything other then NO_SEC. >> >> Second also with none-NO_SEC once a client does a chown the server >> needs to RECALL all other clients, so as Peng says above the Server >> should/can also recall from us. >> >> But some good sole needs to make an errata at the RFC draft so to >> explain. > I'd rather not violate the existing RFC unless we have a good reason for it > and an errata out. Having a flag to return on chown for the object layout > seems like the easier way to go forward. Ack Benny > -- > 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 -- 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 --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 7dd8aca..63a5b80 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -3213,7 +3213,9 @@ 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_SIZE && + sattr->ia_size < i_size_read(inode)) pnfs_commit_and_return_layout(inode); nfs_fattr_init(fattr); diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h index aca3dff..04a3646 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 truncate to + * a smaller size */ PNFS_LAYOUTRET_ON_SETATTR = 1 << 0, PNFS_LAYOUTRET_ON_ERROR = 1 << 1, };
Both blocks layout and objects layout want to use it to avoid CB_LAYOUTRECALL but that should only happen if client is doing truncation to a smaller size. For other cases, we let server decide if it wants to recall client's layouts. 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 | 4 +++- fs/nfs/pnfs.h | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-)