| Message ID | 20211207152705.167010-1-jlayton@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] ceph: don't check for quotas on MDS stray dirs | expand |
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. > 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,
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>
On 12/7/21 11:27 PM, 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> > 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, LGTM. Reviewed-by: Xiubo Li <xiubli@redhat.com>
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,