| Message ID | 20220126173649.163500-1-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] ceph: set pool_ns in new inode layout for async creates | expand |
On Wed, Jan 26, 2022 at 6:36 PM Jeff Layton <jlayton@kernel.org> wrote: > > Dan reported that he was unable to write to files that had been > asynchronously created when the client's OSD caps are restricted to a > particular namespace. > > The issue is that the layout for the new inode is only partially being > filled. Ensure that we populate the pool_ns_data and pool_ns_len in the > iinfo before calling ceph_fill_inode. > > URL: https://tracker.ceph.com/issues/54013 > Reported-by: Dan van der Ster <dan@vanderster.com> > Fixes: 9a8d03ca2e2c ("ceph: attempt to do async create when possible") > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/ceph/file.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > v2: don't take extra reference, just use rcu_dereference_raw > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c > index cbe4d5a5cde5..22ca724aef36 100644 > --- a/fs/ceph/file.c > +++ b/fs/ceph/file.c > @@ -599,6 +599,7 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry, > struct ceph_inode_info *ci = ceph_inode(dir); > struct inode *inode; > struct timespec64 now; > + struct ceph_string *pool_ns; > struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(dir->i_sb); > struct ceph_vino vino = { .ino = req->r_deleg_ino, > .snap = CEPH_NOSNAP }; > @@ -648,6 +649,12 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry, > in.max_size = cpu_to_le64(lo->stripe_unit); > > ceph_file_layout_to_legacy(lo, &in.layout); > + /* lo is private, so pool_ns can't change */ > + pool_ns = rcu_dereference_raw(lo->pool_ns); > + if (pool_ns) { > + iinfo.pool_ns_len = pool_ns->len; > + iinfo.pool_ns_data = pool_ns->str; > + } > > down_read(&mdsc->snap_rwsem); > ret = ceph_fill_inode(inode, NULL, &iinfo, NULL, req->r_session, > -- > 2.34.1 > There seems to be a gap in nowsync coverage -- do we have a ticket for adding a test case for namespace-constrained caps or someone is already on it? Reviewed-by: Ilya Dryomov <idryomov@gmail.com> Thanks, Ilya
On Wed, 2022-01-26 at 20:10 +0100, Ilya Dryomov wrote: > On Wed, Jan 26, 2022 at 6:36 PM Jeff Layton <jlayton@kernel.org> wrote: > > > > Dan reported that he was unable to write to files that had been > > asynchronously created when the client's OSD caps are restricted to a > > particular namespace. > > > > The issue is that the layout for the new inode is only partially being > > filled. Ensure that we populate the pool_ns_data and pool_ns_len in the > > iinfo before calling ceph_fill_inode. > > > > URL: https://tracker.ceph.com/issues/54013 > > Reported-by: Dan van der Ster <dan@vanderster.com> > > Fixes: 9a8d03ca2e2c ("ceph: attempt to do async create when possible") > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > --- > > fs/ceph/file.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > v2: don't take extra reference, just use rcu_dereference_raw > > > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c > > index cbe4d5a5cde5..22ca724aef36 100644 > > --- a/fs/ceph/file.c > > +++ b/fs/ceph/file.c > > @@ -599,6 +599,7 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry, > > struct ceph_inode_info *ci = ceph_inode(dir); > > struct inode *inode; > > struct timespec64 now; > > + struct ceph_string *pool_ns; > > struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(dir->i_sb); > > struct ceph_vino vino = { .ino = req->r_deleg_ino, > > .snap = CEPH_NOSNAP }; > > @@ -648,6 +649,12 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry, > > in.max_size = cpu_to_le64(lo->stripe_unit); > > > > ceph_file_layout_to_legacy(lo, &in.layout); > > + /* lo is private, so pool_ns can't change */ > > + pool_ns = rcu_dereference_raw(lo->pool_ns); > > + if (pool_ns) { > > + iinfo.pool_ns_len = pool_ns->len; > > + iinfo.pool_ns_data = pool_ns->str; > > + } > > > > down_read(&mdsc->snap_rwsem); > > ret = ceph_fill_inode(inode, NULL, &iinfo, NULL, req->r_session, > > -- > > 2.34.1 > > > > There seems to be a gap in nowsync coverage -- do we have a ticket for > adding a test case for namespace-constrained caps or someone is already > on it? > > Reviewed-by: Ilya Dryomov <idryomov@gmail.com> > Yeah, I was planning to look into that as part of the tracker that Dan opened. We do test both wsync and nowsync with the kclient, but apparently we don't test namespace-constrained caps with them? Or maybe we just got unlucky with some of the randomness involved? I'm not sure either way, but this configuration is essentially what "ceph fs subvolume authorize" does, so we need to ensure that we are testing it regularly.
diff --git a/fs/ceph/file.c b/fs/ceph/file.c index cbe4d5a5cde5..22ca724aef36 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -599,6 +599,7 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry, struct ceph_inode_info *ci = ceph_inode(dir); struct inode *inode; struct timespec64 now; + struct ceph_string *pool_ns; struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(dir->i_sb); struct ceph_vino vino = { .ino = req->r_deleg_ino, .snap = CEPH_NOSNAP }; @@ -648,6 +649,12 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry, in.max_size = cpu_to_le64(lo->stripe_unit); ceph_file_layout_to_legacy(lo, &in.layout); + /* lo is private, so pool_ns can't change */ + pool_ns = rcu_dereference_raw(lo->pool_ns); + if (pool_ns) { + iinfo.pool_ns_len = pool_ns->len; + iinfo.pool_ns_data = pool_ns->str; + } down_read(&mdsc->snap_rwsem); ret = ceph_fill_inode(inode, NULL, &iinfo, NULL, req->r_session,
Dan reported that he was unable to write to files that had been asynchronously created when the client's OSD caps are restricted to a particular namespace. The issue is that the layout for the new inode is only partially being filled. Ensure that we populate the pool_ns_data and pool_ns_len in the iinfo before calling ceph_fill_inode. URL: https://tracker.ceph.com/issues/54013 Reported-by: Dan van der Ster <dan@vanderster.com> Fixes: 9a8d03ca2e2c ("ceph: attempt to do async create when possible") Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/ceph/file.c | 7 +++++++ 1 file changed, 7 insertions(+) v2: don't take extra reference, just use rcu_dereference_raw