Re: [PATCH v2] ceph: don't check for quotas on MDS stray dirs

["Luís Henriques" <lhenriques@suse.de>]

On Tue, Dec 07, 2021 at 10:29:28AM -0500, Jeff Layton wrote:
> On Tue, 2021-12-07 at 10:27 -0500, Jeff Layton wrote:
> > 玮文 胡 reported seeing the WARN_RATELIMIT pop when writing to an
> > inode that had been transplanted into the stray dir. The client was
> > trying to look up the quotarealm info from the parent and that tripped
> > the warning.
> > 
> > Change the ceph_vino_is_reserved helper to not throw a warning for
> > MDS stray directories (0x100 - 0x1ff), only for reserved dirs that
> > are not in that range.
> > 
> > Also, fix ceph_has_realms_with_quotas to return false when encountering
> > a reserved inode.
> > 
> > URL: https://tracker.ceph.com/issues/53180
> > Reported-by: Hu Weiwen <sehuww@mail.scut.edu.cn>
> > Reviewed-by: Luis Henriques <lhenriques@suse.de>
> > Reviewed-by: Xiubo Li <xiubli@redhat.com>
> 
> Oops, I forgot to remote the Reviewed-by: lines before sending since
> this patch is different. The one in the testing branch has them removed.
> Reviews of this patch would still be welcome.

Feel free to add my Reviewed-by: back, the patch looks OK to me.  Thanks!

Cheers,
--
Luís

> 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/ceph/quota.c |  3 +++
> >  fs/ceph/super.h | 20 ++++++++++++--------
> >  2 files changed, 15 insertions(+), 8 deletions(-)
> > 
> > I was still seeing some warnings even with the earlier patch, so I
> > decided to rework it to just never warn on MDS stray dirs. This should
> > also silence the warnings on MDS stray dirs (and also alleviate Luis'
> > concern about the function renaming in the earlier patch).
> > 
> > diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c
> > index 24ae13ea2241..a338a3ec0dc4 100644
> > --- a/fs/ceph/quota.c
> > +++ b/fs/ceph/quota.c
> > @@ -30,6 +30,9 @@ static inline bool ceph_has_realms_with_quotas(struct inode *inode)
> >  	/* if root is the real CephFS root, we don't have quota realms */
> >  	if (root && ceph_ino(root) == CEPH_INO_ROOT)
> >  		return false;
> > +	/* MDS stray dirs have no quota realms */
> > +	if (ceph_vino_is_reserved(ceph_inode(inode)->i_vino))
> > +		return false;
> >  	/* otherwise, we can't know for sure */
> >  	return true;
> >  }
> > diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> > index 387ee33894db..f9b1bbf26c1b 100644
> > --- a/fs/ceph/super.h
> > +++ b/fs/ceph/super.h
> > @@ -541,19 +541,23 @@ static inline int ceph_ino_compare(struct inode *inode, void *data)
> >   *
> >   * These come from src/mds/mdstypes.h in the ceph sources.
> >   */
> > -#define CEPH_MAX_MDS		0x100
> > -#define CEPH_NUM_STRAY		10
> > +#define CEPH_MAX_MDS			0x100
> > +#define CEPH_NUM_STRAY			10
> >  #define CEPH_MDS_INO_MDSDIR_OFFSET	(1 * CEPH_MAX_MDS)
> > +#define CEPH_MDS_INO_LOG_OFFSET		(2 * CEPH_MAX_MDS)
> >  #define CEPH_INO_SYSTEM_BASE		((6*CEPH_MAX_MDS) + (CEPH_MAX_MDS * CEPH_NUM_STRAY))
> >  
> >  static inline bool ceph_vino_is_reserved(const struct ceph_vino vino)
> >  {
> > -	if (vino.ino < CEPH_INO_SYSTEM_BASE &&
> > -	    vino.ino >= CEPH_MDS_INO_MDSDIR_OFFSET) {
> > -		WARN_RATELIMIT(1, "Attempt to access reserved inode number 0x%llx", vino.ino);
> > -		return true;
> > -	}
> > -	return false;
> > +	if (vino.ino >= CEPH_INO_SYSTEM_BASE ||
> > +	    vino.ino < CEPH_MDS_INO_MDSDIR_OFFSET)
> > +		return false;
> > +
> > +	/* Don't warn on mdsdirs */
> > +	WARN_RATELIMIT(vino.ino >= CEPH_MDS_INO_LOG_OFFSET,
> > +			"Attempt to access reserved inode number 0x%llx",
> > +			vino.ino);
> > +	return true;
> >  }
> >  
> >  static inline struct inode *ceph_find_inode(struct super_block *sb,
> 
> -- 
> Jeff Layton <jlayton@kernel.org>