| Message ID | 20220425195427.60738-1-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: fix setting of xattrs on async created inodes | expand |
On Mon, 2022-04-25 at 15:54 -0400, Jeff Layton wrote: > Currently when we create a file, we spin up an xattr buffer to send > along with the create request. If we end up doing an async create > however, then we currently pass down a zero-length xattr buffer. > > Fix the code to send down the xattr buffer in req->r_pagelist. If the > xattrs span more than a page, however give up and don't try to do an > async create. > > Fixes: 9a8d03ca2e2c ("ceph: attempt to do async create when possible") > URL: https://bugzilla.redhat.com/show_bug.cgi?id=2063929 > Reported-by: John Fortin <fortinj66@gmail.com> > Reported-by: Sri Ramanujam <sri@ramanujam.io> > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/ceph/file.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c > index 6c9e837aa1d3..8c8226c0feac 100644 > --- a/fs/ceph/file.c > +++ b/fs/ceph/file.c > @@ -629,9 +629,15 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry, > iinfo.change_attr = 1; > ceph_encode_timespec64(&iinfo.btime, &now); > > - iinfo.xattr_len = ARRAY_SIZE(xattr_buf); > - iinfo.xattr_data = xattr_buf; > - memset(iinfo.xattr_data, 0, iinfo.xattr_len); > + if (req->r_pagelist) { > + iinfo.xattr_len = req->r_pagelist->length; > + iinfo.xattr_data = req->r_pagelist->mapped_tail; > + } else { > + /* fake it */ > + iinfo.xattr_len = ARRAY_SIZE(xattr_buf); > + iinfo.xattr_data = xattr_buf; > + memset(iinfo.xattr_data, 0, iinfo.xattr_len); > + } > > in.ino = cpu_to_le64(vino.ino); > in.snapid = cpu_to_le64(CEPH_NOSNAP); > @@ -743,6 +749,10 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry, > err = ceph_security_init_secctx(dentry, mode, &as_ctx); > if (err < 0) > goto out_ctx; > + /* Async create can't handle more than a page of xattrs */ > + if (as_ctx.pagelist && > + !list_is_singular(&as_ctx.pagelist->head)) > + try_async = false; > } else if (!d_in_lookup(dentry)) { > /* If it's not being looked up, it's negative */ > return -ENOENT; Oh, I meant to mark this for stable as well. Xiubo, can you do that when you merge it? Thanks,
On 4/26/22 3:57 AM, Jeff Layton wrote: > On Mon, 2022-04-25 at 15:54 -0400, Jeff Layton wrote: >> Currently when we create a file, we spin up an xattr buffer to send >> along with the create request. If we end up doing an async create >> however, then we currently pass down a zero-length xattr buffer. >> >> Fix the code to send down the xattr buffer in req->r_pagelist. If the >> xattrs span more than a page, however give up and don't try to do an >> async create. >> >> Fixes: 9a8d03ca2e2c ("ceph: attempt to do async create when possible") >> URL: https://bugzilla.redhat.com/show_bug.cgi?id=2063929 >> Reported-by: John Fortin <fortinj66@gmail.com> >> Reported-by: Sri Ramanujam <sri@ramanujam.io> >> Signed-off-by: Jeff Layton <jlayton@kernel.org> >> --- >> fs/ceph/file.c | 16 +++++++++++++--- >> 1 file changed, 13 insertions(+), 3 deletions(-) >> >> diff --git a/fs/ceph/file.c b/fs/ceph/file.c >> index 6c9e837aa1d3..8c8226c0feac 100644 >> --- a/fs/ceph/file.c >> +++ b/fs/ceph/file.c >> @@ -629,9 +629,15 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry, >> iinfo.change_attr = 1; >> ceph_encode_timespec64(&iinfo.btime, &now); >> >> - iinfo.xattr_len = ARRAY_SIZE(xattr_buf); >> - iinfo.xattr_data = xattr_buf; >> - memset(iinfo.xattr_data, 0, iinfo.xattr_len); >> + if (req->r_pagelist) { >> + iinfo.xattr_len = req->r_pagelist->length; >> + iinfo.xattr_data = req->r_pagelist->mapped_tail; >> + } else { >> + /* fake it */ >> + iinfo.xattr_len = ARRAY_SIZE(xattr_buf); >> + iinfo.xattr_data = xattr_buf; >> + memset(iinfo.xattr_data, 0, iinfo.xattr_len); >> + } >> >> in.ino = cpu_to_le64(vino.ino); >> in.snapid = cpu_to_le64(CEPH_NOSNAP); >> @@ -743,6 +749,10 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry, >> err = ceph_security_init_secctx(dentry, mode, &as_ctx); >> if (err < 0) >> goto out_ctx; >> + /* Async create can't handle more than a page of xattrs */ >> + if (as_ctx.pagelist && >> + !list_is_singular(&as_ctx.pagelist->head)) >> + try_async = false; >> } else if (!d_in_lookup(dentry)) { >> /* If it's not being looked up, it's negative */ >> return -ENOENT; > Oh, I meant to mark this for stable as well. Xiubo, can you do that when > you merge it? Sure, Jeff. Looks nice. Merged into the testing branch. Thanks -- Xiubo Looks good. I fixed up the minor spelling error in the comment that Venky noticed too. Merged into testing branch. Thanks, > Thanks,
diff --git a/fs/ceph/file.c b/fs/ceph/file.c index 6c9e837aa1d3..8c8226c0feac 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -629,9 +629,15 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry, iinfo.change_attr = 1; ceph_encode_timespec64(&iinfo.btime, &now); - iinfo.xattr_len = ARRAY_SIZE(xattr_buf); - iinfo.xattr_data = xattr_buf; - memset(iinfo.xattr_data, 0, iinfo.xattr_len); + if (req->r_pagelist) { + iinfo.xattr_len = req->r_pagelist->length; + iinfo.xattr_data = req->r_pagelist->mapped_tail; + } else { + /* fake it */ + iinfo.xattr_len = ARRAY_SIZE(xattr_buf); + iinfo.xattr_data = xattr_buf; + memset(iinfo.xattr_data, 0, iinfo.xattr_len); + } in.ino = cpu_to_le64(vino.ino); in.snapid = cpu_to_le64(CEPH_NOSNAP); @@ -743,6 +749,10 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry, err = ceph_security_init_secctx(dentry, mode, &as_ctx); if (err < 0) goto out_ctx; + /* Async create can't handle more than a page of xattrs */ + if (as_ctx.pagelist && + !list_is_singular(&as_ctx.pagelist->head)) + try_async = false; } else if (!d_in_lookup(dentry)) { /* If it's not being looked up, it's negative */ return -ENOENT;
Currently when we create a file, we spin up an xattr buffer to send along with the create request. If we end up doing an async create however, then we currently pass down a zero-length xattr buffer. Fix the code to send down the xattr buffer in req->r_pagelist. If the xattrs span more than a page, however give up and don't try to do an async create. Fixes: 9a8d03ca2e2c ("ceph: attempt to do async create when possible") URL: https://bugzilla.redhat.com/show_bug.cgi?id=2063929 Reported-by: John Fortin <fortinj66@gmail.com> Reported-by: Sri Ramanujam <sri@ramanujam.io> Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/ceph/file.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)