2020-05-21 09:41:59

by Yanhu Cao

[permalink] [raw]
Subject: [PATCH] ceph: show max caps in debugfs caps file

before
------
total 1026
avail 1024
used 2
reserved 0
min 1024

after
------
total 1026
avail 1024
used 2
max 2048
reserved 0
min 1024

Signed-off-by: Yanhu Cao <[email protected]>
---
fs/ceph/caps.c | 6 ++++--
fs/ceph/debugfs.c | 7 ++++---
fs/ceph/super.h | 2 +-
3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 5f3aa4d607de..e2c759a2ef35 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -404,8 +404,8 @@ void ceph_put_cap(struct ceph_mds_client *mdsc, struct ceph_cap *cap)
}

void ceph_reservation_status(struct ceph_fs_client *fsc,
- int *total, int *avail, int *used, int *reserved,
- int *min)
+ int *total, int *avail, int *used, int *max,
+ int *reserved, int *min)
{
struct ceph_mds_client *mdsc = fsc->mdsc;

@@ -417,6 +417,8 @@ void ceph_reservation_status(struct ceph_fs_client *fsc,
*avail = mdsc->caps_avail_count;
if (used)
*used = mdsc->caps_use_count;
+ if (max)
+ *max = mdsc->caps_use_max;
if (reserved)
*reserved = mdsc->caps_reserve_count;
if (min)
diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 481ac97b4d25..942004376588 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -138,16 +138,17 @@ static int caps_show(struct seq_file *s, void *p)
{
struct ceph_fs_client *fsc = s->private;
struct ceph_mds_client *mdsc = fsc->mdsc;
- int total, avail, used, reserved, min, i;
+ int total, avail, used, max, reserved, min, i;
struct cap_wait *cw;

- ceph_reservation_status(fsc, &total, &avail, &used, &reserved, &min);
+ ceph_reservation_status(fsc, &total, &avail, &used, &max,
+ &reserved, &min);
seq_printf(s, "total\t\t%d\n"
"avail\t\t%d\n"
"used\t\t%d\n"
"reserved\t%d\n"
"min\t\t%d\n\n",
- total, avail, used, reserved, min);
+ total, avail, used, max, reserved, min);
seq_printf(s, "ino issued implemented\n");
seq_printf(s, "-----------------------------------------------\n");

diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 60aac3aee055..79aa42d9336c 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -700,7 +700,7 @@ extern void ceph_unreserve_caps(struct ceph_mds_client *mdsc,
struct ceph_cap_reservation *ctx);
extern void ceph_reservation_status(struct ceph_fs_client *client,
int *total, int *avail, int *used,
- int *reserved, int *min);
+ int *max, int *reserved, int *min);



--
2.24.2 (Apple Git-127)


2020-05-21 11:11:53

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH] ceph: show max caps in debugfs caps file

On Thu, 2020-05-21 at 17:38 +0800, Yanhu Cao wrote:
> before
> ------
> total 1026
> avail 1024
> used 2
> reserved 0
> min 1024
>
> after
> ------
> total 1026
> avail 1024
> used 2
> max 2048
> reserved 0
> min 1024
>
> Signed-off-by: Yanhu Cao <[email protected]>
> ---
> fs/ceph/caps.c | 6 ++++--
> fs/ceph/debugfs.c | 7 ++++---
> fs/ceph/super.h | 2 +-
> 3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 5f3aa4d607de..e2c759a2ef35 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -404,8 +404,8 @@ void ceph_put_cap(struct ceph_mds_client *mdsc, struct ceph_cap *cap)
> }
>
> void ceph_reservation_status(struct ceph_fs_client *fsc,
> - int *total, int *avail, int *used, int *reserved,
> - int *min)
> + int *total, int *avail, int *used, int *max,
> + int *reserved, int *min)
> {
> struct ceph_mds_client *mdsc = fsc->mdsc;
>
> @@ -417,6 +417,8 @@ void ceph_reservation_status(struct ceph_fs_client *fsc,
> *avail = mdsc->caps_avail_count;
> if (used)
> *used = mdsc->caps_use_count;
> + if (max)
> + *max = mdsc->caps_use_max;

Can you lay out what value this will provide? I'm not convinced that
this information is really that helpful:

mdsc->caps_use_max is just set to the value of the "caps_max" mount
option, and that information is displayed in /proc/mounts if it's not
set to the default.

What might be more interesting is to track the most recent "max_caps"
value sent by the MDS (see the CEPH_SESSION_RECALL_STATE message
handling). Tracking that would give us a more dynamic view of the
current maximum requested by the MDS, which is often going to be less
than what "caps_max" was set to at mount time.

> if (reserved)
> *reserved = mdsc->caps_reserve_count;
> if (min)
> diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
> index 481ac97b4d25..942004376588 100644
> --- a/fs/ceph/debugfs.c
> +++ b/fs/ceph/debugfs.c
> @@ -138,16 +138,17 @@ static int caps_show(struct seq_file *s, void *p)
> {
> struct ceph_fs_client *fsc = s->private;
> struct ceph_mds_client *mdsc = fsc->mdsc;
> - int total, avail, used, reserved, min, i;
> + int total, avail, used, max, reserved, min, i;
> struct cap_wait *cw;
>
> - ceph_reservation_status(fsc, &total, &avail, &used, &reserved, &min);
> + ceph_reservation_status(fsc, &total, &avail, &used, &max,
> + &reserved, &min);
> seq_printf(s, "total\t\t%d\n"
> "avail\t\t%d\n"
> "used\t\t%d\n"
> "reserved\t%d\n"
> "min\t\t%d\n\n",
> - total, avail, used, reserved, min);
> + total, avail, used, max, reserved, min);
> seq_printf(s, "ino issued implemented\n");
> seq_printf(s, "-----------------------------------------------\n");
>
> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> index 60aac3aee055..79aa42d9336c 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -700,7 +700,7 @@ extern void ceph_unreserve_caps(struct ceph_mds_client *mdsc,
> struct ceph_cap_reservation *ctx);
> extern void ceph_reservation_status(struct ceph_fs_client *client,
> int *total, int *avail, int *used,
> - int *reserved, int *min);
> + int *max, int *reserved, int *min);
>
>
>

--
Jeff Layton <[email protected]>

2020-05-21 12:21:09

by Yanhu Cao

[permalink] [raw]
Subject: Re: [PATCH] ceph: show max caps in debugfs caps file

On Thu, May 21, 2020 at 7:09 PM Jeff Layton <[email protected]> wrote:
>
> On Thu, 2020-05-21 at 17:38 +0800, Yanhu Cao wrote:
> > before
> > ------
> > total 1026
> > avail 1024
> > used 2
> > reserved 0
> > min 1024
> >
> > after
> > ------
> > total 1026
> > avail 1024
> > used 2
> > max 2048
> > reserved 0
> > min 1024
> >
> > Signed-off-by: Yanhu Cao <[email protected]>
> > ---
> > fs/ceph/caps.c | 6 ++++--
> > fs/ceph/debugfs.c | 7 ++++---
> > fs/ceph/super.h | 2 +-
> > 3 files changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> > index 5f3aa4d607de..e2c759a2ef35 100644
> > --- a/fs/ceph/caps.c
> > +++ b/fs/ceph/caps.c
> > @@ -404,8 +404,8 @@ void ceph_put_cap(struct ceph_mds_client *mdsc, struct ceph_cap *cap)
> > }
> >
> > void ceph_reservation_status(struct ceph_fs_client *fsc,
> > - int *total, int *avail, int *used, int *reserved,
> > - int *min)
> > + int *total, int *avail, int *used, int *max,
> > + int *reserved, int *min)
> > {
> > struct ceph_mds_client *mdsc = fsc->mdsc;
> >
> > @@ -417,6 +417,8 @@ void ceph_reservation_status(struct ceph_fs_client *fsc,
> > *avail = mdsc->caps_avail_count;
> > if (used)
> > *used = mdsc->caps_use_count;
> > + if (max)
> > + *max = mdsc->caps_use_max;
>
> Can you lay out what value this will provide? I'm not convinced that
> this information is really that helpful:
>
> mdsc->caps_use_max is just set to the value of the "caps_max" mount
> option, and that information is displayed in /proc/mounts if it's not
> set to the default.
>
> What might be more interesting is to track the most recent "max_caps"
> value sent by the MDS (see the CEPH_SESSION_RECALL_STATE message
> handling). Tracking that would give us a more dynamic view of the
> current maximum requested by the MDS, which is often going to be less
> than what "caps_max" was set to at mount time.

Do you mean the 'mds_recall_max_caps'? which can be set by the MDS.
Clients use this value every time to trim caps.

There is an option mds_max_caps_per_client which is a soft limit,
which is determined by the behavior of the client.
and we recently encounter a warning '1 MDSs report oversized
cache'(ceph-v12.2.12: mds_cache_memory_limit=64G, used=100G),
Therefore, the effect is not good.

So we want to know whether the caps held by the client exceed caps_max
through the debugfs caps file (default or mount option).


>
> > if (reserved)
> > *reserved = mdsc->caps_reserve_count;
> > if (min)
> > diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
> > index 481ac97b4d25..942004376588 100644
> > --- a/fs/ceph/debugfs.c
> > +++ b/fs/ceph/debugfs.c
> > @@ -138,16 +138,17 @@ static int caps_show(struct seq_file *s, void *p)
> > {
> > struct ceph_fs_client *fsc = s->private;
> > struct ceph_mds_client *mdsc = fsc->mdsc;
> > - int total, avail, used, reserved, min, i;
> > + int total, avail, used, max, reserved, min, i;
> > struct cap_wait *cw;
> >
> > - ceph_reservation_status(fsc, &total, &avail, &used, &reserved, &min);
> > + ceph_reservation_status(fsc, &total, &avail, &used, &max,
> > + &reserved, &min);
> > seq_printf(s, "total\t\t%d\n"
> > "avail\t\t%d\n"
> > "used\t\t%d\n"
> > "reserved\t%d\n"
> > "min\t\t%d\n\n",
> > - total, avail, used, reserved, min);
> > + total, avail, used, max, reserved, min);
> > seq_printf(s, "ino issued implemented\n");
> > seq_printf(s, "-----------------------------------------------\n");
> >
> > diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> > index 60aac3aee055..79aa42d9336c 100644
> > --- a/fs/ceph/super.h
> > +++ b/fs/ceph/super.h
> > @@ -700,7 +700,7 @@ extern void ceph_unreserve_caps(struct ceph_mds_client *mdsc,
> > struct ceph_cap_reservation *ctx);
> > extern void ceph_reservation_status(struct ceph_fs_client *client,
> > int *total, int *avail, int *used,
> > - int *reserved, int *min);
> > + int *max, int *reserved, int *min);
> >
> >
> >
>
> --
> Jeff Layton <[email protected]>
>

2020-05-21 12:56:14

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH] ceph: show max caps in debugfs caps file

On Thu, 2020-05-21 at 20:19 +0800, Yanhu Cao wrote:
> On Thu, May 21, 2020 at 7:09 PM Jeff Layton <[email protected]> wrote:
> > On Thu, 2020-05-21 at 17:38 +0800, Yanhu Cao wrote:
> > > before
> > > ------
> > > total 1026
> > > avail 1024
> > > used 2
> > > reserved 0
> > > min 1024
> > >
> > > after
> > > ------
> > > total 1026
> > > avail 1024
> > > used 2
> > > max 2048
> > > reserved 0
> > > min 1024
> > >
> > > Signed-off-by: Yanhu Cao <[email protected]>
> > > ---
> > > fs/ceph/caps.c | 6 ++++--
> > > fs/ceph/debugfs.c | 7 ++++---
> > > fs/ceph/super.h | 2 +-
> > > 3 files changed, 9 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> > > index 5f3aa4d607de..e2c759a2ef35 100644
> > > --- a/fs/ceph/caps.c
> > > +++ b/fs/ceph/caps.c
> > > @@ -404,8 +404,8 @@ void ceph_put_cap(struct ceph_mds_client *mdsc, struct ceph_cap *cap)
> > > }
> > >
> > > void ceph_reservation_status(struct ceph_fs_client *fsc,
> > > - int *total, int *avail, int *used, int *reserved,
> > > - int *min)
> > > + int *total, int *avail, int *used, int *max,
> > > + int *reserved, int *min)
> > > {
> > > struct ceph_mds_client *mdsc = fsc->mdsc;
> > >
> > > @@ -417,6 +417,8 @@ void ceph_reservation_status(struct ceph_fs_client *fsc,
> > > *avail = mdsc->caps_avail_count;
> > > if (used)
> > > *used = mdsc->caps_use_count;
> > > + if (max)
> > > + *max = mdsc->caps_use_max;
> >
> > Can you lay out what value this will provide? I'm not convinced that
> > this information is really that helpful:
> >
> > mdsc->caps_use_max is just set to the value of the "caps_max" mount
> > option, and that information is displayed in /proc/mounts if it's not
> > set to the default.
> >
> > What might be more interesting is to track the most recent "max_caps"
> > value sent by the MDS (see the CEPH_SESSION_RECALL_STATE message
> > handling). Tracking that would give us a more dynamic view of the
> > current maximum requested by the MDS, which is often going to be less
> > than what "caps_max" was set to at mount time.
>
> Do you mean the 'mds_recall_max_caps'? which can be set by the MDS.
> Clients use this value every time to trim caps.
>
> There is an option mds_max_caps_per_client which is a soft limit,
> which is determined by the behavior of the client.
> and we recently encounter a warning '1 MDSs report oversized
> cache'(ceph-v12.2.12: mds_cache_memory_limit=64G, used=100G),
> Therefore, the effect is not good.
>
> So we want to know whether the caps held by the client exceed caps_max
> through the debugfs caps file (default or mount option).
>
>

Ok, I doubt this patch is going to tell you what you want to know then.
There are two limits involved here:

1/ the limit set by the caps_max mount option for the client
2/ a dynamic limit that is managed by the MDS issuing
CEPH_SESSION_RECALL_STATE messages to the clients (max_caps).

This patch is only going to tell you about the first one, but the second
one is more interesting to monitor on a long-term basis.

When the mds exceeds its memory limits, it can issue
CEPH_SESSION_RECALL_STATE messages to the clients, to tell them to
reduce their own caches to a particular size. Currently we just take
that value and feed it into ceph_trim_caps and forget about it.

It would probably be useful though to keep track of the most recent
value issued by the MDS, and print this value as min(mount_option_max,
max_caps_from_mds). Bonus points if you can help untangle the confusing
naming of all these values in the process.

> > > if (reserved)
> > > *reserved = mdsc->caps_reserve_count;
> > > if (min)
> > > diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
> > > index 481ac97b4d25..942004376588 100644
> > > --- a/fs/ceph/debugfs.c
> > > +++ b/fs/ceph/debugfs.c
> > > @@ -138,16 +138,17 @@ static int caps_show(struct seq_file *s, void *p)
> > > {
> > > struct ceph_fs_client *fsc = s->private;
> > > struct ceph_mds_client *mdsc = fsc->mdsc;
> > > - int total, avail, used, reserved, min, i;
> > > + int total, avail, used, max, reserved, min, i;
> > > struct cap_wait *cw;
> > >
> > > - ceph_reservation_status(fsc, &total, &avail, &used, &reserved, &min);
> > > + ceph_reservation_status(fsc, &total, &avail, &used, &max,
> > > + &reserved, &min);
> > > seq_printf(s, "total\t\t%d\n"
> > > "avail\t\t%d\n"
> > > "used\t\t%d\n"
> > > "reserved\t%d\n"
> > > "min\t\t%d\n\n",
> > > - total, avail, used, reserved, min);
> > > + total, avail, used, max, reserved, min);
> > > seq_printf(s, "ino issued implemented\n");
> > > seq_printf(s, "-----------------------------------------------\n");
> > >
> > > diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> > > index 60aac3aee055..79aa42d9336c 100644
> > > --- a/fs/ceph/super.h
> > > +++ b/fs/ceph/super.h
> > > @@ -700,7 +700,7 @@ extern void ceph_unreserve_caps(struct ceph_mds_client *mdsc,
> > > struct ceph_cap_reservation *ctx);
> > > extern void ceph_reservation_status(struct ceph_fs_client *client,
> > > int *total, int *avail, int *used,
> > > - int *reserved, int *min);
> > > + int *max, int *reserved, int *min);
> > >
> > >
> > >
> >
> > --
> > Jeff Layton <[email protected]>
> >

--
Jeff Layton <[email protected]>

2020-05-21 16:06:44

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] ceph: show max caps in debugfs caps file

Hi Yanhu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on ceph-client/for-linus]
[also build test WARNING on v5.7-rc6 next-20200519]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Yanhu-Cao/ceph-show-max-caps-in-debugfs-caps-file/20200521-190841
base: https://github.com/ceph/ceph-client.git for-linus
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <[email protected]>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

fs/ceph/debugfs.c: In function 'caps_show':
>> fs/ceph/debugfs.c:146:16: warning: too many arguments for format [-Wformat-extra-args]
146 | seq_printf(s, "totaltt%dn"
| ^~~~~~~~~~~~~~~

vim +146 fs/ceph/debugfs.c

ff4a80bf2d3f80 Jeff Layton 2019-04-24 136
76aa844d5b2fb8 Sage Weil 2009-10-06 137 static int caps_show(struct seq_file *s, void *p)
76aa844d5b2fb8 Sage Weil 2009-10-06 138 {
3d14c5d2b6e15c Yehuda Sadeh 2010-04-06 139 struct ceph_fs_client *fsc = s->private;
ff4a80bf2d3f80 Jeff Layton 2019-04-24 140 struct ceph_mds_client *mdsc = fsc->mdsc;
d35440110f623b Yanhu Cao 2020-05-21 141 int total, avail, used, max, reserved, min, i;
3a3430affce5de Jeff Layton 2019-11-20 142 struct cap_wait *cw;
76aa844d5b2fb8 Sage Weil 2009-10-06 143
d35440110f623b Yanhu Cao 2020-05-21 144 ceph_reservation_status(fsc, &total, &avail, &used, &max,
d35440110f623b Yanhu Cao 2020-05-21 145 &reserved, &min);
76aa844d5b2fb8 Sage Weil 2009-10-06 @146 seq_printf(s, "total\t\t%d\n"
76aa844d5b2fb8 Sage Weil 2009-10-06 147 "avail\t\t%d\n"
76aa844d5b2fb8 Sage Weil 2009-10-06 148 "used\t\t%d\n"
85ccce43a3fc15 Sage Weil 2010-02-17 149 "reserved\t%d\n"
ff4a80bf2d3f80 Jeff Layton 2019-04-24 150 "min\t\t%d\n\n",
d35440110f623b Yanhu Cao 2020-05-21 151 total, avail, used, max, reserved, min);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 152 seq_printf(s, "ino issued implemented\n");
ff4a80bf2d3f80 Jeff Layton 2019-04-24 153 seq_printf(s, "-----------------------------------------------\n");
ff4a80bf2d3f80 Jeff Layton 2019-04-24 154
ff4a80bf2d3f80 Jeff Layton 2019-04-24 155 mutex_lock(&mdsc->mutex);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 156 for (i = 0; i < mdsc->max_sessions; i++) {
ff4a80bf2d3f80 Jeff Layton 2019-04-24 157 struct ceph_mds_session *session;
ff4a80bf2d3f80 Jeff Layton 2019-04-24 158
ff4a80bf2d3f80 Jeff Layton 2019-04-24 159 session = __ceph_lookup_mds_session(mdsc, i);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 160 if (!session)
ff4a80bf2d3f80 Jeff Layton 2019-04-24 161 continue;
ff4a80bf2d3f80 Jeff Layton 2019-04-24 162 mutex_unlock(&mdsc->mutex);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 163 mutex_lock(&session->s_mutex);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 164 ceph_iterate_session_caps(session, caps_show_cb, s);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 165 mutex_unlock(&session->s_mutex);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 166 ceph_put_mds_session(session);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 167 mutex_lock(&mdsc->mutex);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 168 }
ff4a80bf2d3f80 Jeff Layton 2019-04-24 169 mutex_unlock(&mdsc->mutex);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 170
3a3430affce5de Jeff Layton 2019-11-20 171 seq_printf(s, "\n\nWaiters:\n--------\n");
3a3430affce5de Jeff Layton 2019-11-20 172 seq_printf(s, "tgid ino need want\n");
3a3430affce5de Jeff Layton 2019-11-20 173 seq_printf(s, "-----------------------------------------------------\n");
3a3430affce5de Jeff Layton 2019-11-20 174
3a3430affce5de Jeff Layton 2019-11-20 175 spin_lock(&mdsc->caps_list_lock);
3a3430affce5de Jeff Layton 2019-11-20 176 list_for_each_entry(cw, &mdsc->cap_wait_list, list) {
3a3430affce5de Jeff Layton 2019-11-20 177 seq_printf(s, "%-13d0x%-17lx%-17s%-17s\n", cw->tgid, cw->ino,
3a3430affce5de Jeff Layton 2019-11-20 178 ceph_cap_string(cw->need),
3a3430affce5de Jeff Layton 2019-11-20 179 ceph_cap_string(cw->want));
3a3430affce5de Jeff Layton 2019-11-20 180 }
3a3430affce5de Jeff Layton 2019-11-20 181 spin_unlock(&mdsc->caps_list_lock);
3a3430affce5de Jeff Layton 2019-11-20 182
76aa844d5b2fb8 Sage Weil 2009-10-06 183 return 0;
76aa844d5b2fb8 Sage Weil 2009-10-06 184 }
76aa844d5b2fb8 Sage Weil 2009-10-06 185

:::::: The code at line 146 was first introduced by commit
:::::: 76aa844d5b2fb8c839180d3f5874e333b297e5fd ceph: debugfs

:::::: TO: Sage Weil <[email protected]>
:::::: CC: Sage Weil <[email protected]>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (5.41 kB)
.config.gz (61.91 kB)
Download all attachments

2020-05-22 08:29:22

by Yanhu Cao

[permalink] [raw]
Subject: Re: [PATCH] ceph: show max caps in debugfs caps file

On Thu, May 21, 2020 at 8:51 PM Jeff Layton <[email protected]> wrote:
>
> On Thu, 2020-05-21 at 20:19 +0800, Yanhu Cao wrote:
> > On Thu, May 21, 2020 at 7:09 PM Jeff Layton <[email protected]> wrote:
> > > On Thu, 2020-05-21 at 17:38 +0800, Yanhu Cao wrote:
> > > > before
> > > > ------
> > > > total 1026
> > > > avail 1024
> > > > used 2
> > > > reserved 0
> > > > min 1024
> > > >
> > > > after
> > > > ------
> > > > total 1026
> > > > avail 1024
> > > > used 2
> > > > max 2048
> > > > reserved 0
> > > > min 1024
> > > >
> > > > Signed-off-by: Yanhu Cao <[email protected]>
> > > > ---
> > > > fs/ceph/caps.c | 6 ++++--
> > > > fs/ceph/debugfs.c | 7 ++++---
> > > > fs/ceph/super.h | 2 +-
> > > > 3 files changed, 9 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> > > > index 5f3aa4d607de..e2c759a2ef35 100644
> > > > --- a/fs/ceph/caps.c
> > > > +++ b/fs/ceph/caps.c
> > > > @@ -404,8 +404,8 @@ void ceph_put_cap(struct ceph_mds_client *mdsc, struct ceph_cap *cap)
> > > > }
> > > >
> > > > void ceph_reservation_status(struct ceph_fs_client *fsc,
> > > > - int *total, int *avail, int *used, int *reserved,
> > > > - int *min)
> > > > + int *total, int *avail, int *used, int *max,
> > > > + int *reserved, int *min)
> > > > {
> > > > struct ceph_mds_client *mdsc = fsc->mdsc;
> > > >
> > > > @@ -417,6 +417,8 @@ void ceph_reservation_status(struct ceph_fs_client *fsc,
> > > > *avail = mdsc->caps_avail_count;
> > > > if (used)
> > > > *used = mdsc->caps_use_count;
> > > > + if (max)
> > > > + *max = mdsc->caps_use_max;
> > >
> > > Can you lay out what value this will provide? I'm not convinced that
> > > this information is really that helpful:
> > >
> > > mdsc->caps_use_max is just set to the value of the "caps_max" mount
> > > option, and that information is displayed in /proc/mounts if it's not
> > > set to the default.
> > >
> > > What might be more interesting is to track the most recent "max_caps"
> > > value sent by the MDS (see the CEPH_SESSION_RECALL_STATE message
> > > handling). Tracking that would give us a more dynamic view of the
> > > current maximum requested by the MDS, which is often going to be less
> > > than what "caps_max" was set to at mount time.
> >
> > Do you mean the 'mds_recall_max_caps'? which can be set by the MDS.
> > Clients use this value every time to trim caps.
> >
> > There is an option mds_max_caps_per_client which is a soft limit,
> > which is determined by the behavior of the client.
> > and we recently encounter a warning '1 MDSs report oversized
> > cache'(ceph-v12.2.12: mds_cache_memory_limit=64G, used=100G),
> > Therefore, the effect is not good.
> >
> > So we want to know whether the caps held by the client exceed caps_max
> > through the debugfs caps file (default or mount option).
> >
> >
>
> Ok, I doubt this patch is going to tell you what you want to know then.
> There are two limits involved here:
>
> 1/ the limit set by the caps_max mount option for the client
> 2/ a dynamic limit that is managed by the MDS issuing
> CEPH_SESSION_RECALL_STATE messages to the clients (max_caps).
>
> This patch is only going to tell you about the first one, but the second
> one is more interesting to monitor on a long-term basis.
>
> When the mds exceeds its memory limits, it can issue
> CEPH_SESSION_RECALL_STATE messages to the clients, to tell them to
> reduce their own caches to a particular size. Currently we just take
> that value and feed it into ceph_trim_caps and forget about it.
>
> It would probably be useful though to keep track of the most recent
> value issued by the MDS, and print this value as min(mount_option_max,
> max_caps_from_mds). Bonus points if you can help untangle the confusing
> naming of all these values in the process.

How about this? Add a new field caps_limit(calculated by
session->caps, mds_recall_max_caps,
mds_max_caps_per_client, mds_min_caps_per_client) in ceph_mds_client,
which can be set by ceph_trim_caps.

struct ceph_mds_client {
...
- int caps_use_max; /* max used caps */
+ int caps_use_max; /* max used caps, limited
by client */
+ int caps_limit; /* limited by mds */
...
}

int ceph_trim_caps(...)
{
int trim_caps = session->s_nr_caps - max_caps;
+ mdsc->caps_limit = max_caps;
...
}

if client's caps have no limit, we can track caps_limit.
Don't use min(caps_use_max, caps_limit) because it shows more clearly
whether it is limited by the client or mds.

e.g.
----
total 3112
avail 1025
used 2087
limit 2068 => caps_limit
max 2048 => caps_use_max(mount_option_caps_max)

trimmed
-------
total 1943
avail 1025
used 918
limit 918
max 2048


>
> > > > if (reserved)
> > > > *reserved = mdsc->caps_reserve_count;
> > > > if (min)
> > > > diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
> > > > index 481ac97b4d25..942004376588 100644
> > > > --- a/fs/ceph/debugfs.c
> > > > +++ b/fs/ceph/debugfs.c
> > > > @@ -138,16 +138,17 @@ static int caps_show(struct seq_file *s, void *p)
> > > > {
> > > > struct ceph_fs_client *fsc = s->private;
> > > > struct ceph_mds_client *mdsc = fsc->mdsc;
> > > > - int total, avail, used, reserved, min, i;
> > > > + int total, avail, used, max, reserved, min, i;
> > > > struct cap_wait *cw;
> > > >
> > > > - ceph_reservation_status(fsc, &total, &avail, &used, &reserved, &min);
> > > > + ceph_reservation_status(fsc, &total, &avail, &used, &max,
> > > > + &reserved, &min);
> > > > seq_printf(s, "total\t\t%d\n"
> > > > "avail\t\t%d\n"
> > > > "used\t\t%d\n"
> > > > "reserved\t%d\n"
> > > > "min\t\t%d\n\n",
> > > > - total, avail, used, reserved, min);
> > > > + total, avail, used, max, reserved, min);
> > > > seq_printf(s, "ino issued implemented\n");
> > > > seq_printf(s, "-----------------------------------------------\n");
> > > >
> > > > diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> > > > index 60aac3aee055..79aa42d9336c 100644
> > > > --- a/fs/ceph/super.h
> > > > +++ b/fs/ceph/super.h
> > > > @@ -700,7 +700,7 @@ extern void ceph_unreserve_caps(struct ceph_mds_client *mdsc,
> > > > struct ceph_cap_reservation *ctx);
> > > > extern void ceph_reservation_status(struct ceph_fs_client *client,
> > > > int *total, int *avail, int *used,
> > > > - int *reserved, int *min);
> > > > + int *max, int *reserved, int *min);
> > > >
> > > >
> > > >
> > >
> > > --
> > > Jeff Layton <[email protected]>
> > >
>
> --
> Jeff Layton <[email protected]>
>

2020-05-22 11:29:02

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH] ceph: show max caps in debugfs caps file

On Fri, 2020-05-22 at 16:27 +0800, Yanhu Cao wrote:
> On Thu, May 21, 2020 at 8:51 PM Jeff Layton <[email protected]> wrote:
> > On Thu, 2020-05-21 at 20:19 +0800, Yanhu Cao wrote:
> > > On Thu, May 21, 2020 at 7:09 PM Jeff Layton <[email protected]> wrote:
> > > > On Thu, 2020-05-21 at 17:38 +0800, Yanhu Cao wrote:
> > > > > before
> > > > > ------
> > > > > total 1026
> > > > > avail 1024
> > > > > used 2
> > > > > reserved 0
> > > > > min 1024
> > > > >
> > > > > after
> > > > > ------
> > > > > total 1026
> > > > > avail 1024
> > > > > used 2
> > > > > max 2048
> > > > > reserved 0
> > > > > min 1024
> > > > >
> > > > > Signed-off-by: Yanhu Cao <[email protected]>
> > > > > ---
> > > > > fs/ceph/caps.c | 6 ++++--
> > > > > fs/ceph/debugfs.c | 7 ++++---
> > > > > fs/ceph/super.h | 2 +-
> > > > > 3 files changed, 9 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> > > > > index 5f3aa4d607de..e2c759a2ef35 100644
> > > > > --- a/fs/ceph/caps.c
> > > > > +++ b/fs/ceph/caps.c
> > > > > @@ -404,8 +404,8 @@ void ceph_put_cap(struct ceph_mds_client *mdsc, struct ceph_cap *cap)
> > > > > }
> > > > >
> > > > > void ceph_reservation_status(struct ceph_fs_client *fsc,
> > > > > - int *total, int *avail, int *used, int *reserved,
> > > > > - int *min)
> > > > > + int *total, int *avail, int *used, int *max,
> > > > > + int *reserved, int *min)
> > > > > {
> > > > > struct ceph_mds_client *mdsc = fsc->mdsc;
> > > > >
> > > > > @@ -417,6 +417,8 @@ void ceph_reservation_status(struct ceph_fs_client *fsc,
> > > > > *avail = mdsc->caps_avail_count;
> > > > > if (used)
> > > > > *used = mdsc->caps_use_count;
> > > > > + if (max)
> > > > > + *max = mdsc->caps_use_max;
> > > >
> > > > Can you lay out what value this will provide? I'm not convinced that
> > > > this information is really that helpful:
> > > >
> > > > mdsc->caps_use_max is just set to the value of the "caps_max" mount
> > > > option, and that information is displayed in /proc/mounts if it's not
> > > > set to the default.
> > > >
> > > > What might be more interesting is to track the most recent "max_caps"
> > > > value sent by the MDS (see the CEPH_SESSION_RECALL_STATE message
> > > > handling). Tracking that would give us a more dynamic view of the
> > > > current maximum requested by the MDS, which is often going to be less
> > > > than what "caps_max" was set to at mount time.
> > >
> > > Do you mean the 'mds_recall_max_caps'? which can be set by the MDS.
> > > Clients use this value every time to trim caps.
> > >
> > > There is an option mds_max_caps_per_client which is a soft limit,
> > > which is determined by the behavior of the client.
> > > and we recently encounter a warning '1 MDSs report oversized
> > > cache'(ceph-v12.2.12: mds_cache_memory_limit=64G, used=100G),
> > > Therefore, the effect is not good.
> > >
> > > So we want to know whether the caps held by the client exceed caps_max
> > > through the debugfs caps file (default or mount option).
> > >
> > >
> >
> > Ok, I doubt this patch is going to tell you what you want to know then.
> > There are two limits involved here:
> >
> > 1/ the limit set by the caps_max mount option for the client
> > 2/ a dynamic limit that is managed by the MDS issuing
> > CEPH_SESSION_RECALL_STATE messages to the clients (max_caps).
> >
> > This patch is only going to tell you about the first one, but the second
> > one is more interesting to monitor on a long-term basis.
> >
> > When the mds exceeds its memory limits, it can issue
> > CEPH_SESSION_RECALL_STATE messages to the clients, to tell them to
> > reduce their own caches to a particular size. Currently we just take
> > that value and feed it into ceph_trim_caps and forget about it.
> >
> > It would probably be useful though to keep track of the most recent
> > value issued by the MDS, and print this value as min(mount_option_max,
> > max_caps_from_mds). Bonus points if you can help untangle the confusing
> > naming of all these values in the process.
>
> How about this? Add a new field caps_limit(calculated by
> session->caps, mds_recall_max_caps,
> mds_max_caps_per_client, mds_min_caps_per_client) in ceph_mds_client,
> which can be set by ceph_trim_caps.
>
> struct ceph_mds_client {
> ...
> - int caps_use_max; /* max used caps */
> + int caps_use_max; /* max used caps, limited
> by client */
> + int caps_limit; /* limited by mds */
> ...
> }
>
> int ceph_trim_caps(...)
> {
> int trim_caps = session->s_nr_caps - max_caps;
> + mdsc->caps_limit = max_caps;
> ...
> }
>
> if client's caps have no limit, we can track caps_limit.
> Don't use min(caps_use_max, caps_limit) because it shows more clearly
> whether it is limited by the client or mds.
>
> e.g.
> ----
> total 3112
> avail 1025
> used 2087
> limit 2068 => caps_limit
> max 2048 => caps_use_max(mount_option_caps_max)
>
> trimmed
> -------
> total 1943
> avail 1025
> used 918
> limit 918
> max 2048
>
>

Sure, that'd be fine. The "max" is somewhat redundant info that you can
get elsewhere, but printing it here won't hurt anything.

> > > > > if (reserved)
> > > > > *reserved = mdsc->caps_reserve_count;
> > > > > if (min)
> > > > > diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
> > > > > index 481ac97b4d25..942004376588 100644
> > > > > --- a/fs/ceph/debugfs.c
> > > > > +++ b/fs/ceph/debugfs.c
> > > > > @@ -138,16 +138,17 @@ static int caps_show(struct seq_file *s, void *p)
> > > > > {
> > > > > struct ceph_fs_client *fsc = s->private;
> > > > > struct ceph_mds_client *mdsc = fsc->mdsc;
> > > > > - int total, avail, used, reserved, min, i;
> > > > > + int total, avail, used, max, reserved, min, i;
> > > > > struct cap_wait *cw;
> > > > >
> > > > > - ceph_reservation_status(fsc, &total, &avail, &used, &reserved, &min);
> > > > > + ceph_reservation_status(fsc, &total, &avail, &used, &max,
> > > > > + &reserved, &min);
> > > > > seq_printf(s, "total\t\t%d\n"
> > > > > "avail\t\t%d\n"
> > > > > "used\t\t%d\n"
> > > > > "reserved\t%d\n"
> > > > > "min\t\t%d\n\n",
> > > > > - total, avail, used, reserved, min);
> > > > > + total, avail, used, max, reserved, min);
> > > > > seq_printf(s, "ino issued implemented\n");
> > > > > seq_printf(s, "-----------------------------------------------\n");
> > > > >
> > > > > diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> > > > > index 60aac3aee055..79aa42d9336c 100644
> > > > > --- a/fs/ceph/super.h
> > > > > +++ b/fs/ceph/super.h
> > > > > @@ -700,7 +700,7 @@ extern void ceph_unreserve_caps(struct ceph_mds_client *mdsc,
> > > > > struct ceph_cap_reservation *ctx);
> > > > > extern void ceph_reservation_status(struct ceph_fs_client *client,
> > > > > int *total, int *avail, int *used,
> > > > > - int *reserved, int *min);
> > > > > + int *max, int *reserved, int *min);
> > > > >
> > > > >
> > > > >
> > > >
> > > > --
> > > > Jeff Layton <[email protected]>
> > > >
> >
> > --
> > Jeff Layton <[email protected]>
> >

--
Jeff Layton <[email protected]>

2020-06-02 09:49:15

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] ceph: show max caps in debugfs caps file

Hi Yanhu,

url: https://github.com/0day-ci/linux/commits/Yanhu-Cao/ceph-show-max-caps-in-debugfs-caps-file/20200521-190841
base: https://github.com/ceph/ceph-client.git for-linus
config: x86_64-randconfig-m001-20200529 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>

smatch warnings:
fs/ceph/debugfs.c:151 caps_show() warn: excess argument passed to 'seq_printf'

# https://github.com/0day-ci/linux/commit/d35440110f623b99d201f15fa3cc062a35dc6373
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout d35440110f623b99d201f15fa3cc062a35dc6373
vim +/seq_printf +151 fs/ceph/debugfs.c

76aa844d5b2fb8 Sage Weil 2009-10-06 137 static int caps_show(struct seq_file *s, void *p)
76aa844d5b2fb8 Sage Weil 2009-10-06 138 {
3d14c5d2b6e15c Yehuda Sadeh 2010-04-06 139 struct ceph_fs_client *fsc = s->private;
ff4a80bf2d3f80 Jeff Layton 2019-04-24 140 struct ceph_mds_client *mdsc = fsc->mdsc;
d35440110f623b Yanhu Cao 2020-05-21 141 int total, avail, used, max, reserved, min, i;
3a3430affce5de Jeff Layton 2019-11-20 142 struct cap_wait *cw;
76aa844d5b2fb8 Sage Weil 2009-10-06 143
d35440110f623b Yanhu Cao 2020-05-21 144 ceph_reservation_status(fsc, &total, &avail, &used, &max,
d35440110f623b Yanhu Cao 2020-05-21 145 &reserved, &min);
76aa844d5b2fb8 Sage Weil 2009-10-06 146 seq_printf(s, "total\t\t%d\n"
76aa844d5b2fb8 Sage Weil 2009-10-06 147 "avail\t\t%d\n"
76aa844d5b2fb8 Sage Weil 2009-10-06 148 "used\t\t%d\n"

max is missing.

85ccce43a3fc15 Sage Weil 2010-02-17 149 "reserved\t%d\n"
ff4a80bf2d3f80 Jeff Layton 2019-04-24 150 "min\t\t%d\n\n",
d35440110f623b Yanhu Cao 2020-05-21 @151 total, avail, used, max, reserved, min);
^^^

ff4a80bf2d3f80 Jeff Layton 2019-04-24 152 seq_printf(s, "ino issued implemented\n");
ff4a80bf2d3f80 Jeff Layton 2019-04-24 153 seq_printf(s, "-----------------------------------------------\n");
ff4a80bf2d3f80 Jeff Layton 2019-04-24 154
ff4a80bf2d3f80 Jeff Layton 2019-04-24 155 mutex_lock(&mdsc->mutex);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 156 for (i = 0; i < mdsc->max_sessions; i++) {
ff4a80bf2d3f80 Jeff Layton 2019-04-24 157 struct ceph_mds_session *session;
ff4a80bf2d3f80 Jeff Layton 2019-04-24 158
ff4a80bf2d3f80 Jeff Layton 2019-04-24 159 session = __ceph_lookup_mds_session(mdsc, i);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 160 if (!session)
ff4a80bf2d3f80 Jeff Layton 2019-04-24 161 continue;
ff4a80bf2d3f80 Jeff Layton 2019-04-24 162 mutex_unlock(&mdsc->mutex);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 163 mutex_lock(&session->s_mutex);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 164 ceph_iterate_session_caps(session, caps_show_cb, s);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 165 mutex_unlock(&session->s_mutex);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 166 ceph_put_mds_session(session);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 167 mutex_lock(&mdsc->mutex);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 168 }
ff4a80bf2d3f80 Jeff Layton 2019-04-24 169 mutex_unlock(&mdsc->mutex);
ff4a80bf2d3f80 Jeff Layton 2019-04-24 170
3a3430affce5de Jeff Layton 2019-11-20 171 seq_printf(s, "\n\nWaiters:\n--------\n");
3a3430affce5de Jeff Layton 2019-11-20 172 seq_printf(s, "tgid ino need want\n");
3a3430affce5de Jeff Layton 2019-11-20 173 seq_printf(s, "-----------------------------------------------------\n");
3a3430affce5de Jeff Layton 2019-11-20 174
3a3430affce5de Jeff Layton 2019-11-20 175 spin_lock(&mdsc->caps_list_lock);
3a3430affce5de Jeff Layton 2019-11-20 176 list_for_each_entry(cw, &mdsc->cap_wait_list, list) {
3a3430affce5de Jeff Layton 2019-11-20 177 seq_printf(s, "%-13d0x%-17lx%-17s%-17s\n", cw->tgid, cw->ino,
3a3430affce5de Jeff Layton 2019-11-20 178 ceph_cap_string(cw->need),
3a3430affce5de Jeff Layton 2019-11-20 179 ceph_cap_string(cw->want));
3a3430affce5de Jeff Layton 2019-11-20 180 }
3a3430affce5de Jeff Layton 2019-11-20 181 spin_unlock(&mdsc->caps_list_lock);
3a3430affce5de Jeff Layton 2019-11-20 182
76aa844d5b2fb8 Sage Weil 2009-10-06 183 return 0;
76aa844d5b2fb8 Sage Weil 2009-10-06 184 }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (4.65 kB)
.config.gz (36.50 kB)
Download all attachments