* [PATCH] ceph: set pool_ns in new inode layout for async creates
@ 2022-01-25 21:10 Jeff Layton
2022-01-26 16:23 ` Ilya Dryomov
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Layton @ 2022-01-25 21:10 UTC (permalink / raw)
To: ceph-devel; +Cc: idryomov, Dan van der Ster
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.
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(+)
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index cbe4d5a5cde5..efea321ff643 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,11 +649,17 @@ 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);
+ pool_ns = ceph_try_get_string(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,
req->r_fmode, NULL);
up_read(&mdsc->snap_rwsem);
+ ceph_put_string(pool_ns);
if (ret) {
dout("%s failed to fill inode: %d\n", __func__, ret);
ceph_dir_clear_complete(dir);
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ceph: set pool_ns in new inode layout for async creates
2022-01-25 21:10 [PATCH] ceph: set pool_ns in new inode layout for async creates Jeff Layton
@ 2022-01-26 16:23 ` Ilya Dryomov
2022-01-26 16:34 ` Jeff Layton
0 siblings, 1 reply; 4+ messages in thread
From: Ilya Dryomov @ 2022-01-26 16:23 UTC (permalink / raw)
To: Jeff Layton; +Cc: Ceph Development, Dan van der Ster
On Tue, Jan 25, 2022 at 10:10 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.
>
> 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(+)
>
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index cbe4d5a5cde5..efea321ff643 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,11 +649,17 @@ 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);
> + pool_ns = ceph_try_get_string(lo->pool_ns);
> + if (pool_ns) {
> + iinfo.pool_ns_len = pool_ns->len;
> + iinfo.pool_ns_data = pool_ns->str;
> + }
Considering that we have a reference from try_prep_async_create(), do
we actually need to bother with ceph_try_get_string() here?
Thanks,
Ilya
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ceph: set pool_ns in new inode layout for async creates
2022-01-26 16:23 ` Ilya Dryomov
@ 2022-01-26 16:34 ` Jeff Layton
2022-01-26 16:46 ` Ilya Dryomov
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Layton @ 2022-01-26 16:34 UTC (permalink / raw)
To: Ilya Dryomov; +Cc: Ceph Development, Dan van der Ster
On Wed, 2022-01-26 at 17:23 +0100, Ilya Dryomov wrote:
> On Tue, Jan 25, 2022 at 10:10 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.
> >
> > 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(+)
> >
> > diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> > index cbe4d5a5cde5..efea321ff643 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,11 +649,17 @@ 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);
> > + pool_ns = ceph_try_get_string(lo->pool_ns);
> > + if (pool_ns) {
> > + iinfo.pool_ns_len = pool_ns->len;
> > + iinfo.pool_ns_data = pool_ns->str;
> > + }
>
> Considering that we have a reference from try_prep_async_create(), do
> we actually need to bother with ceph_try_get_string() here?
>
Technically, no. We could just do a rcu_dereference_protected there
since we know that lo is private and can't change. Want me to send a v2?
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ceph: set pool_ns in new inode layout for async creates
2022-01-26 16:34 ` Jeff Layton
@ 2022-01-26 16:46 ` Ilya Dryomov
0 siblings, 0 replies; 4+ messages in thread
From: Ilya Dryomov @ 2022-01-26 16:46 UTC (permalink / raw)
To: Jeff Layton; +Cc: Ceph Development, Dan van der Ster
On Wed, Jan 26, 2022 at 5:34 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> On Wed, 2022-01-26 at 17:23 +0100, Ilya Dryomov wrote:
> > On Tue, Jan 25, 2022 at 10:10 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.
> > >
> > > 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(+)
> > >
> > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> > > index cbe4d5a5cde5..efea321ff643 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,11 +649,17 @@ 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);
> > > + pool_ns = ceph_try_get_string(lo->pool_ns);
> > > + if (pool_ns) {
> > > + iinfo.pool_ns_len = pool_ns->len;
> > > + iinfo.pool_ns_data = pool_ns->str;
> > > + }
> >
> > Considering that we have a reference from try_prep_async_create(), do
> > we actually need to bother with ceph_try_get_string() here?
> >
>
> Technically, no. We could just do a rcu_dereference_protected there
> since we know that lo is private and can't change. Want me to send a v2?
Yeah, let's not do the reference dance when it isn't needed.
I'd probably use rcu_dereference_raw() to avoid having to think about
omitted READ_ONCE (even if the reasoning is trivial), but up to you.
Thanks,
Ilya
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-26 16:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-25 21:10 [PATCH] ceph: set pool_ns in new inode layout for async creates Jeff Layton
2022-01-26 16:23 ` Ilya Dryomov
2022-01-26 16:34 ` Jeff Layton
2022-01-26 16:46 ` Ilya Dryomov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).