* [PATCH] ceph: don't check for quotas on MDS stray dirs
@ 2021-11-09 17:10 Jeff Layton
2021-11-09 18:21 ` Luís Henriques
2021-11-15 2:56 ` Xiubo Li
0 siblings, 2 replies; 3+ messages in thread
From: Jeff Layton @ 2021-11-09 17:10 UTC (permalink / raw)
To: ceph-devel; +Cc: idryomov, lhenriques
玮文 胡 reported seeing the WARN_RATELIMITED 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 and
add a new ceph_vino_warn_reserved() helper that does. Change all of the
existing callsites to call the "warn" variant, and have
ceph_has_realms_with_quotas check return false when the vino is
reserved.
URL: https://tracker.ceph.com/issues/53180
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
fs/ceph/export.c | 4 ++--
fs/ceph/inode.c | 2 +-
fs/ceph/quota.c | 3 +++
fs/ceph/super.h | 17 ++++++++++-------
4 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/fs/ceph/export.c b/fs/ceph/export.c
index e0fa66ac8b9f..a75cf07d668f 100644
--- a/fs/ceph/export.c
+++ b/fs/ceph/export.c
@@ -130,7 +130,7 @@ static struct inode *__lookup_inode(struct super_block *sb, u64 ino)
vino.ino = ino;
vino.snap = CEPH_NOSNAP;
- if (ceph_vino_is_reserved(vino))
+ if (ceph_vino_warn_reserved(vino))
return ERR_PTR(-ESTALE);
inode = ceph_find_inode(sb, vino);
@@ -224,7 +224,7 @@ static struct dentry *__snapfh_to_dentry(struct super_block *sb,
vino.snap = sfh->snapid;
}
- if (ceph_vino_is_reserved(vino))
+ if (ceph_vino_warn_reserved(vino))
return ERR_PTR(-ESTALE);
inode = ceph_find_inode(sb, vino);
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index e8eb8612ddd6..a685fab56772 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -56,7 +56,7 @@ struct inode *ceph_get_inode(struct super_block *sb, struct ceph_vino vino)
{
struct inode *inode;
- if (ceph_vino_is_reserved(vino))
+ if (ceph_vino_warn_reserved(vino))
return ERR_PTR(-EREMOTEIO);
inode = iget5_locked(sb, (unsigned long)vino.ino, ceph_ino_compare,
diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c
index 620c691af40e..d1158c40bb0c 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 ed51e04739c4..c232ed8e8a37 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -547,18 +547,21 @@ static inline int ceph_ino_compare(struct inode *inode, void *data)
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;
+ return vino.ino < CEPH_INO_SYSTEM_BASE &&
+ vino.ino >= CEPH_MDS_INO_MDSDIR_OFFSET;
+}
+
+static inline bool ceph_vino_warn_reserved(const struct ceph_vino vino)
+{
+ return WARN_RATELIMIT(ceph_vino_is_reserved(vino),
+ "Attempt to access reserved inode number 0x%llx",
+ vino.ino);
}
static inline struct inode *ceph_find_inode(struct super_block *sb,
struct ceph_vino vino)
{
- if (ceph_vino_is_reserved(vino))
+ if (ceph_vino_warn_reserved(vino))
return NULL;
/*
--
2.33.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ceph: don't check for quotas on MDS stray dirs
2021-11-09 17:10 [PATCH] ceph: don't check for quotas on MDS stray dirs Jeff Layton
@ 2021-11-09 18:21 ` Luís Henriques
2021-11-15 2:56 ` Xiubo Li
1 sibling, 0 replies; 3+ messages in thread
From: Luís Henriques @ 2021-11-09 18:21 UTC (permalink / raw)
To: Jeff Layton; +Cc: ceph-devel, idryomov
On Tue, Nov 09, 2021 at 12:10:11PM -0500, Jeff Layton wrote:
> 玮文 胡 reported seeing the WARN_RATELIMITED 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 and
> add a new ceph_vino_warn_reserved() helper that does. Change all of the
> existing callsites to call the "warn" variant, and have
> ceph_has_realms_with_quotas check return false when the vino is
> reserved.
>
> URL: https://tracker.ceph.com/issues/53180
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> fs/ceph/export.c | 4 ++--
> fs/ceph/inode.c | 2 +-
> fs/ceph/quota.c | 3 +++
> fs/ceph/super.h | 17 ++++++++++-------
> 4 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ceph/export.c b/fs/ceph/export.c
> index e0fa66ac8b9f..a75cf07d668f 100644
> --- a/fs/ceph/export.c
> +++ b/fs/ceph/export.c
> @@ -130,7 +130,7 @@ static struct inode *__lookup_inode(struct super_block *sb, u64 ino)
> vino.ino = ino;
> vino.snap = CEPH_NOSNAP;
>
> - if (ceph_vino_is_reserved(vino))
> + if (ceph_vino_warn_reserved(vino))
> return ERR_PTR(-ESTALE);
>
> inode = ceph_find_inode(sb, vino);
> @@ -224,7 +224,7 @@ static struct dentry *__snapfh_to_dentry(struct super_block *sb,
> vino.snap = sfh->snapid;
> }
>
> - if (ceph_vino_is_reserved(vino))
> + if (ceph_vino_warn_reserved(vino))
> return ERR_PTR(-ESTALE);
>
> inode = ceph_find_inode(sb, vino);
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index e8eb8612ddd6..a685fab56772 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -56,7 +56,7 @@ struct inode *ceph_get_inode(struct super_block *sb, struct ceph_vino vino)
> {
> struct inode *inode;
>
> - if (ceph_vino_is_reserved(vino))
> + if (ceph_vino_warn_reserved(vino))
> return ERR_PTR(-EREMOTEIO);
>
> inode = iget5_locked(sb, (unsigned long)vino.ino, ceph_ino_compare,
> diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c
> index 620c691af40e..d1158c40bb0c 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 ed51e04739c4..c232ed8e8a37 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -547,18 +547,21 @@ static inline int ceph_ino_compare(struct inode *inode, void *data)
>
> 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;
> + return vino.ino < CEPH_INO_SYSTEM_BASE &&
> + vino.ino >= CEPH_MDS_INO_MDSDIR_OFFSET;
> +}
> +
> +static inline bool ceph_vino_warn_reserved(const struct ceph_vino vino)
> +{
> + return WARN_RATELIMIT(ceph_vino_is_reserved(vino),
> + "Attempt to access reserved inode number 0x%llx",
> + vino.ino);
> }
>
> static inline struct inode *ceph_find_inode(struct super_block *sb,
> struct ceph_vino vino)
> {
> - if (ceph_vino_is_reserved(vino))
> + if (ceph_vino_warn_reserved(vino))
> return NULL;
>
> /*
> --
> 2.33.1
>
This looks reasonable. I would probably keep the old function name and
simply name the new one ceph_vino_is_reserved_no_warn(). But that's just
because I'm lazy :-)
Reviewed-by: Luis Henriques <lhenriques@suse.de>
Cheers,
--
Luís
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ceph: don't check for quotas on MDS stray dirs
2021-11-09 17:10 [PATCH] ceph: don't check for quotas on MDS stray dirs Jeff Layton
2021-11-09 18:21 ` Luís Henriques
@ 2021-11-15 2:56 ` Xiubo Li
1 sibling, 0 replies; 3+ messages in thread
From: Xiubo Li @ 2021-11-15 2:56 UTC (permalink / raw)
To: Jeff Layton, ceph-devel; +Cc: idryomov, lhenriques
On 11/10/21 1:10 AM, Jeff Layton wrote:
> 玮文 胡 reported seeing the WARN_RATELIMITED 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 and
> add a new ceph_vino_warn_reserved() helper that does. Change all of the
> existing callsites to call the "warn" variant, and have
> ceph_has_realms_with_quotas check return false when the vino is
> reserved.
>
> URL: https://tracker.ceph.com/issues/53180
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> fs/ceph/export.c | 4 ++--
> fs/ceph/inode.c | 2 +-
> fs/ceph/quota.c | 3 +++
> fs/ceph/super.h | 17 ++++++++++-------
> 4 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ceph/export.c b/fs/ceph/export.c
> index e0fa66ac8b9f..a75cf07d668f 100644
> --- a/fs/ceph/export.c
> +++ b/fs/ceph/export.c
> @@ -130,7 +130,7 @@ static struct inode *__lookup_inode(struct super_block *sb, u64 ino)
> vino.ino = ino;
> vino.snap = CEPH_NOSNAP;
>
> - if (ceph_vino_is_reserved(vino))
> + if (ceph_vino_warn_reserved(vino))
> return ERR_PTR(-ESTALE);
>
> inode = ceph_find_inode(sb, vino);
> @@ -224,7 +224,7 @@ static struct dentry *__snapfh_to_dentry(struct super_block *sb,
> vino.snap = sfh->snapid;
> }
>
> - if (ceph_vino_is_reserved(vino))
> + if (ceph_vino_warn_reserved(vino))
> return ERR_PTR(-ESTALE);
>
> inode = ceph_find_inode(sb, vino);
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index e8eb8612ddd6..a685fab56772 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -56,7 +56,7 @@ struct inode *ceph_get_inode(struct super_block *sb, struct ceph_vino vino)
> {
> struct inode *inode;
>
> - if (ceph_vino_is_reserved(vino))
> + if (ceph_vino_warn_reserved(vino))
> return ERR_PTR(-EREMOTEIO);
>
> inode = iget5_locked(sb, (unsigned long)vino.ino, ceph_ino_compare,
> diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c
> index 620c691af40e..d1158c40bb0c 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 ed51e04739c4..c232ed8e8a37 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -547,18 +547,21 @@ static inline int ceph_ino_compare(struct inode *inode, void *data)
>
> 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;
> + return vino.ino < CEPH_INO_SYSTEM_BASE &&
> + vino.ino >= CEPH_MDS_INO_MDSDIR_OFFSET;
> +}
> +
> +static inline bool ceph_vino_warn_reserved(const struct ceph_vino vino)
> +{
> + return WARN_RATELIMIT(ceph_vino_is_reserved(vino),
> + "Attempt to access reserved inode number 0x%llx",
> + vino.ino);
> }
>
> static inline struct inode *ceph_find_inode(struct super_block *sb,
> struct ceph_vino vino)
> {
> - if (ceph_vino_is_reserved(vino))
> + if (ceph_vino_warn_reserved(vino))
> return NULL;
>
> /*
LGTM.
Reviewed-by: Xiubo Li <xiubli@redhat.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-15 2:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-09 17:10 [PATCH] ceph: don't check for quotas on MDS stray dirs Jeff Layton
2021-11-09 18:21 ` Luís Henriques
2021-11-15 2:56 ` Xiubo Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.