| Message ID | 20211025150042.1677-1-lhenriques@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,v2] ceph: add remote object copy counter to fs client metrics | expand |
On Mon, 2021-10-25 at 16:00 +0100, Luís Henriques wrote: > This counter will keep track of the number of remote object copies done on > copy_file_range syscalls. This counter will be kept using the metrics > infrastructure and thus accessible through debugfs. For now, this counter > won't be sent to the MDS. > > Cc: Patrick Donnelly <pdonnell@redhat.com> > Signed-off-by: Luís Henriques <lhenriques@suse.de> > --- > Hi! > > So, here's v2 of this RFC. Now, I guess that Patrick's idea of adding > this counter was to validate the test results, isn't that right? If so, > this has to be done from within the fstest code and not from teuthology > test. The reason is that fstests mount and unmount the filesystems under > test, which effectively wipe the metrics on the client. > > So, the follow-up to this patch would be changes to the corresponding > fstests so that they would access this debugfs file and check the counter > is set to the expected value. > > Cheers, > -- > Luís > > fs/ceph/debugfs.c | 6 ++++++ > fs/ceph/file.c | 1 + > fs/ceph/metric.c | 2 ++ > fs/ceph/metric.h | 2 ++ > 4 files changed, 11 insertions(+) > > diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c > index 38b78b45811f..9f1a09816541 100644 > --- a/fs/ceph/debugfs.c > +++ b/fs/ceph/debugfs.c > @@ -235,6 +235,12 @@ static int metric_show(struct seq_file *s, void *p) > percpu_counter_sum(&m->i_caps_mis), > percpu_counter_sum(&m->i_caps_hit)); > > + seq_printf(s, "\n"); > + seq_printf(s, "item total\n"); > + seq_printf(s, "-------------------\n"); > + seq_printf(s, "%-14s%-16lld\n", "copy-from", > + atomic64_read(&m->total_copyfrom)); > + > return 0; > } > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c > index e61018d9764e..b36a7b9c1ab8 100644 > --- a/fs/ceph/file.c > +++ b/fs/ceph/file.c > @@ -2253,6 +2253,7 @@ static ssize_t ceph_do_objects_copy(struct ceph_inode_info *src_ci, u64 *src_off > bytes = ret; > goto out; > } > + atomic64_inc(&fsc->mdsc->metric.total_copyfrom); > len -= object_size; > bytes += object_size; > *src_off += object_size; > diff --git a/fs/ceph/metric.c b/fs/ceph/metric.c > index 04d5df29bbbf..a8a9f96c56a8 100644 > --- a/fs/ceph/metric.c > +++ b/fs/ceph/metric.c > @@ -278,6 +278,8 @@ int ceph_metric_init(struct ceph_client_metric *m) > if (ret) > goto err_total_inodes; > > + atomic64_set(&m->total_copyfrom, 0); > + > m->session = NULL; > INIT_DELAYED_WORK(&m->delayed_work, metric_delayed_work); > > diff --git a/fs/ceph/metric.h b/fs/ceph/metric.h > index 0133955a3c6a..a1e2cd46de6b 100644 > --- a/fs/ceph/metric.h > +++ b/fs/ceph/metric.h > @@ -169,6 +169,8 @@ struct ceph_client_metric { > struct percpu_counter opened_inodes; > struct percpu_counter total_inodes; > > + atomic64_t total_copyfrom; > + > struct ceph_mds_session *session; > struct delayed_work delayed_work; /* delayed work */ > }; I know the main interest currently is just the count of ops, but I do think that we'll want a full set of stats like we track for reads/writes, and I'd rather not rev the file format any more than we need to. Could you extend struct ceph_client_metric with a full set of copy stats and plumb in the places to record and report them? It should be pretty similar to how reads/writes are tracked.
diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c index 38b78b45811f..9f1a09816541 100644 --- a/fs/ceph/debugfs.c +++ b/fs/ceph/debugfs.c @@ -235,6 +235,12 @@ static int metric_show(struct seq_file *s, void *p) percpu_counter_sum(&m->i_caps_mis), percpu_counter_sum(&m->i_caps_hit)); + seq_printf(s, "\n"); + seq_printf(s, "item total\n"); + seq_printf(s, "-------------------\n"); + seq_printf(s, "%-14s%-16lld\n", "copy-from", + atomic64_read(&m->total_copyfrom)); + return 0; } diff --git a/fs/ceph/file.c b/fs/ceph/file.c index e61018d9764e..b36a7b9c1ab8 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -2253,6 +2253,7 @@ static ssize_t ceph_do_objects_copy(struct ceph_inode_info *src_ci, u64 *src_off bytes = ret; goto out; } + atomic64_inc(&fsc->mdsc->metric.total_copyfrom); len -= object_size; bytes += object_size; *src_off += object_size; diff --git a/fs/ceph/metric.c b/fs/ceph/metric.c index 04d5df29bbbf..a8a9f96c56a8 100644 --- a/fs/ceph/metric.c +++ b/fs/ceph/metric.c @@ -278,6 +278,8 @@ int ceph_metric_init(struct ceph_client_metric *m) if (ret) goto err_total_inodes; + atomic64_set(&m->total_copyfrom, 0); + m->session = NULL; INIT_DELAYED_WORK(&m->delayed_work, metric_delayed_work); diff --git a/fs/ceph/metric.h b/fs/ceph/metric.h index 0133955a3c6a..a1e2cd46de6b 100644 --- a/fs/ceph/metric.h +++ b/fs/ceph/metric.h @@ -169,6 +169,8 @@ struct ceph_client_metric { struct percpu_counter opened_inodes; struct percpu_counter total_inodes; + atomic64_t total_copyfrom; + struct ceph_mds_session *session; struct delayed_work delayed_work; /* delayed work */ };
This counter will keep track of the number of remote object copies done on copy_file_range syscalls. This counter will be kept using the metrics infrastructure and thus accessible through debugfs. For now, this counter won't be sent to the MDS. Cc: Patrick Donnelly <pdonnell@redhat.com> Signed-off-by: Luís Henriques <lhenriques@suse.de> --- Hi! So, here's v2 of this RFC. Now, I guess that Patrick's idea of adding this counter was to validate the test results, isn't that right? If so, this has to be done from within the fstest code and not from teuthology test. The reason is that fstests mount and unmount the filesystems under test, which effectively wipe the metrics on the client. So, the follow-up to this patch would be changes to the corresponding fstests so that they would access this debugfs file and check the counter is set to the expected value. Cheers,