ceph-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Xiubo Li <xiubli@redhat.com>,
	ceph-devel@vger.kernel.org, idryomov@gmail.com
Cc: linux-fsdevel@vger.kernel.org, viro@zeniv.linux.org.uk
Subject: Re: [PATCH 1/2] ceph: don't clobber i_snap_caps on non-I_NEW inode
Date: Fri, 19 Mar 2021 08:51:06 -0400	[thread overview]
Message-ID: <9d87dd0e8b0597caf71863b9321e312e320178e1.camel@kernel.org> (raw)
In-Reply-To: <e70eb841-5669-83e0-4c61-ec8153cc5a9b@redhat.com>

On Fri, 2021-03-19 at 13:03 +0800, Xiubo Li wrote:
> On 2021/3/16 2:07, Jeff Layton wrote:
> > We want the snapdir to mirror the non-snapped directory's attributes for
> > most things, but i_snap_caps represents the caps granted on the snapshot
> > directory by the MDS itself. A misbehaving MDS could issue different
> > caps for the snapdir and we lose them here.
> > 
> > Only reset i_snap_caps when the inode is I_NEW.
> > 
> > Reported-by: Al Viro <viro@zeniv.linux.org.uk>
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >   fs/ceph/inode.c | 7 ++++---
> >   1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > index 26dc7a296f6b..fc7f4bf63306 100644
> > --- a/fs/ceph/inode.c
> > +++ b/fs/ceph/inode.c
> > @@ -101,12 +101,13 @@ struct inode *ceph_get_snapdir(struct inode *parent)
> >   	inode->i_atime = parent->i_atime;
> >   	inode->i_op = &ceph_snapdir_iops;
> >   	inode->i_fop = &ceph_snapdir_fops;
> > -	ci->i_snap_caps = CEPH_CAP_PIN; /* so we can open */
> > -	ci->i_rbytes = 0;
> >   	ci->i_btime = ceph_inode(parent)->i_btime;
> > +	ci->i_rbytes = 0;
> >   
> > 
> > 
> > 
> 
> Hi Jeff,
> 
> BTW, why we need to set other members here if the i_state is not I_NEW ?
> 
> Are they necessary ?
> 

I think so, at least for most of them.

IIUC, we want the .snap directory's metadata to mirror that of the
parent directory, so we want stuff like the ownership and mtime to track
changes in the parent.

I can move the setting of i_op and i_fop into the if block though. Those
should never change anyway, though setting them is harmless here since
we're checking to make sure the type is correct above.

I'll go ahead and do that, but I won't bother re-posting the v2 patch
since it's a trivial change.

> > -	if (inode->i_state & I_NEW)
> > +	if (inode->i_state & I_NEW) {
> > +		ci->i_snap_caps = CEPH_CAP_PIN; /* so we can open */
> >   		unlock_new_inode(inode);
> > +	}
> >   
> > 
> > 
> > 
> >   	return inode;
> >   }
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>


  reply	other threads:[~2021-03-19 12:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-15 18:07 [PATCH 0/2] ceph: snapdir dentry handling fixes Jeff Layton
2021-03-15 18:07 ` [PATCH 1/2] ceph: don't clobber i_snap_caps on non-I_NEW inode Jeff Layton
2021-03-19  5:03   ` Xiubo Li
2021-03-19 12:51     ` Jeff Layton [this message]
2021-03-19 13:42       ` Xiubo Li
2021-03-15 18:07 ` [PATCH 2/2] ceph: don't use d_add in ceph_handle_snapdir Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9d87dd0e8b0597caf71863b9321e312e320178e1.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=ceph-devel@vger.kernel.org \
    --cc=idryomov@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=xiubli@redhat.com \
    --subject='Re: [PATCH 1/2] ceph: don'\''t clobber i_snap_caps on non-I_NEW inode' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).