Re: [PATCH v2] ceph: set pool_ns in new inode layout for async creates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2022-01-26 at 20:10 +0100, Ilya Dryomov wrote:
> On Wed, Jan 26, 2022 at 6:36 PM Jeff Layton <jlayton@xxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> > Fixes: 9a8d03ca2e2c ("ceph: attempt to do async create when possible")
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> >  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@xxxxxxxxx>
> 

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.
-- 
Jeff Layton <jlayton@xxxxxxxxxx>



[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux