| Message ID | 1584510355-6936-1-git-send-email-xiubli@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | ceph: add min/max/stdev latency support | expand |
On Wed, Mar 18, 2020 at 6:46 AM <xiubli@redhat.com> wrote: > > From: Xiubo Li <xiubli@redhat.com> > > Changed in V4: > - fix the 32-bit arches div errors by using DIV64_U64_ROUND_CLOSEST instead. [1/4] > - rebase and combine the stdev patch series [3/4][4/4] > - remove the sum latency showing, which makes no sense for debugging, if it > is really needed in some case then just do (avg * total) in userland. [4/4] > - switch {read/write/metadata}_latency_sum to atomic type since it will be > readed very time when updating the latencies to calculate the stdev. [4/4] > > Changed in V2: > - switch spin lock to cmpxchg [1/4] > > Changed in V3: > - add the __update_min/max_latency helpers [1/4] > > > > # cat /sys/kernel/debug/ceph/0f923fe5-00e6-4866-bf01-2027cb75e94b.client4150/metrics > item total avg_lat(us) min_lat(us) max_lat(us) stdev(us) > ----------------------------------------------------------------------------------- > read 2312 9000 1000 100000 607.4 > write 21777 925000 2000 44551000 29700.3 > metadata 6 4179000 1000 21414000 19590.8 > > item total miss hit > ------------------------------------------------- > d_lease 2 0 11 > caps 2 14 398418 > > > > Xiubo Li (4): > ceph: switch to DIV64_U64_ROUND_CLOSEST to support 32-bit arches > ceph: add min/max latency support for read/write/metadata metrics > ceph: move the metric helpers into one separate file > ceph: add standard deviation support for read/write/metadata perf > metric > > fs/ceph/Makefile | 2 +- > fs/ceph/debugfs.c | 89 ++++++++++++++++++------ > fs/ceph/mds_client.c | 83 +--------------------- > fs/ceph/metric.c | 190 +++++++++++++++++++++++++++++++++++++++++++++++++++ > fs/ceph/metric.h | 79 +++++++++++---------- > 5 files changed, 297 insertions(+), 146 deletions(-) > create mode 100644 fs/ceph/metric.c Hi Xiubo, I think these additions need to be merged with your previous series, so that the history is clean. Ideally the whole thing would start with a single patch adding all of the metrics infrastructure to metric.[ch], followed by patches introducing new metrics and ceph_update_*() calls. Related metrics and ceph_update_*() calls should be added together. No point in splitting read and write OSD latency in two patches as they touch the same functions in addr.c and file.c. Thanks, Ilya
On 2020/3/18 17:11, Ilya Dryomov wrote: > On Wed, Mar 18, 2020 at 6:46 AM <xiubli@redhat.com> wrote: >> From: Xiubo Li <xiubli@redhat.com> >> >> Changed in V4: >> - fix the 32-bit arches div errors by using DIV64_U64_ROUND_CLOSEST instead. [1/4] >> - rebase and combine the stdev patch series [3/4][4/4] >> - remove the sum latency showing, which makes no sense for debugging, if it >> is really needed in some case then just do (avg * total) in userland. [4/4] >> - switch {read/write/metadata}_latency_sum to atomic type since it will be >> readed very time when updating the latencies to calculate the stdev. [4/4] >> >> Changed in V2: >> - switch spin lock to cmpxchg [1/4] >> >> Changed in V3: >> - add the __update_min/max_latency helpers [1/4] >> >> >> >> # cat /sys/kernel/debug/ceph/0f923fe5-00e6-4866-bf01-2027cb75e94b.client4150/metrics >> item total avg_lat(us) min_lat(us) max_lat(us) stdev(us) >> ----------------------------------------------------------------------------------- >> read 2312 9000 1000 100000 607.4 >> write 21777 925000 2000 44551000 29700.3 >> metadata 6 4179000 1000 21414000 19590.8 >> >> item total miss hit >> ------------------------------------------------- >> d_lease 2 0 11 >> caps 2 14 398418 >> >> >> >> Xiubo Li (4): >> ceph: switch to DIV64_U64_ROUND_CLOSEST to support 32-bit arches >> ceph: add min/max latency support for read/write/metadata metrics >> ceph: move the metric helpers into one separate file >> ceph: add standard deviation support for read/write/metadata perf >> metric >> >> fs/ceph/Makefile | 2 +- >> fs/ceph/debugfs.c | 89 ++++++++++++++++++------ >> fs/ceph/mds_client.c | 83 +--------------------- >> fs/ceph/metric.c | 190 +++++++++++++++++++++++++++++++++++++++++++++++++++ >> fs/ceph/metric.h | 79 +++++++++++---------- >> 5 files changed, 297 insertions(+), 146 deletions(-) >> create mode 100644 fs/ceph/metric.c > Hi Xiubo, > > I think these additions need to be merged with your previous series, > so that the history is clean. Ideally the whole thing would start with > a single patch adding all of the metrics infrastructure to metric.[ch], > followed by patches introducing new metrics and ceph_update_*() calls. > > Related metrics and ceph_update_*() calls should be added together. > No point in splitting read and write OSD latency in two patches as they > touch the same functions in addr.c and file.c. Hi Ilya, Yeah, it makes sense and I will merge all the related patch series about the metric and post it again. Thanks BRs > Thanks, > > Ilya >
On Wed, 2020-03-18 at 18:36 +0800, Xiubo Li wrote: > On 2020/3/18 17:11, Ilya Dryomov wrote: > > On Wed, Mar 18, 2020 at 6:46 AM <xiubli@redhat.com> wrote: > > > From: Xiubo Li <xiubli@redhat.com> > > > > > > Changed in V4: > > > - fix the 32-bit arches div errors by using DIV64_U64_ROUND_CLOSEST instead. [1/4] > > > - rebase and combine the stdev patch series [3/4][4/4] > > > - remove the sum latency showing, which makes no sense for debugging, if it > > > is really needed in some case then just do (avg * total) in userland. [4/4] > > > - switch {read/write/metadata}_latency_sum to atomic type since it will be > > > readed very time when updating the latencies to calculate the stdev. [4/4] > > > > > > Changed in V2: > > > - switch spin lock to cmpxchg [1/4] > > > > > > Changed in V3: > > > - add the __update_min/max_latency helpers [1/4] > > > > > > > > > > > > # cat /sys/kernel/debug/ceph/0f923fe5-00e6-4866-bf01-2027cb75e94b.client4150/metrics > > > item total avg_lat(us) min_lat(us) max_lat(us) stdev(us) > > > ----------------------------------------------------------------------------------- > > > read 2312 9000 1000 100000 607.4 > > > write 21777 925000 2000 44551000 29700.3 > > > metadata 6 4179000 1000 21414000 19590.8 > > > > > > item total miss hit > > > ------------------------------------------------- > > > d_lease 2 0 11 > > > caps 2 14 398418 > > > > > > > > > > > > Xiubo Li (4): > > > ceph: switch to DIV64_U64_ROUND_CLOSEST to support 32-bit arches > > > ceph: add min/max latency support for read/write/metadata metrics > > > ceph: move the metric helpers into one separate file > > > ceph: add standard deviation support for read/write/metadata perf > > > metric > > > > > > fs/ceph/Makefile | 2 +- > > > fs/ceph/debugfs.c | 89 ++++++++++++++++++------ > > > fs/ceph/mds_client.c | 83 +--------------------- > > > fs/ceph/metric.c | 190 +++++++++++++++++++++++++++++++++++++++++++++++++++ > > > fs/ceph/metric.h | 79 +++++++++++---------- > > > 5 files changed, 297 insertions(+), 146 deletions(-) > > > create mode 100644 fs/ceph/metric.c > > Hi Xiubo, > > > > I think these additions need to be merged with your previous series, > > so that the history is clean. Ideally the whole thing would start with > > a single patch adding all of the metrics infrastructure to metric.[ch], > > followed by patches introducing new metrics and ceph_update_*() calls. > > > > Related metrics and ceph_update_*() calls should be added together. > > No point in splitting read and write OSD latency in two patches as they > > touch the same functions in addr.c and file.c. > > Hi Ilya, > > Yeah, it makes sense and I will merge all the related patch series about > the metric and post it again. > Sounds good. I've gone ahead and dropped all of the metrics patches from the "testing" branch for now. Please resend the whole series and I'll re-merge them. Thanks,
On 2020/3/18 18:43, Jeff Layton wrote: > On Wed, 2020-03-18 at 18:36 +0800, Xiubo Li wrote: >> On 2020/3/18 17:11, Ilya Dryomov wrote: >>> On Wed, Mar 18, 2020 at 6:46 AM <xiubli@redhat.com> wrote: >>>> From: Xiubo Li <xiubli@redhat.com> >>>> >>>> Changed in V4: >>>> - fix the 32-bit arches div errors by using DIV64_U64_ROUND_CLOSEST instead. [1/4] >>>> - rebase and combine the stdev patch series [3/4][4/4] >>>> - remove the sum latency showing, which makes no sense for debugging, if it >>>> is really needed in some case then just do (avg * total) in userland. [4/4] >>>> - switch {read/write/metadata}_latency_sum to atomic type since it will be >>>> readed very time when updating the latencies to calculate the stdev. [4/4] >>>> >>>> Changed in V2: >>>> - switch spin lock to cmpxchg [1/4] >>>> >>>> Changed in V3: >>>> - add the __update_min/max_latency helpers [1/4] >>>> >>>> >>>> >>>> # cat /sys/kernel/debug/ceph/0f923fe5-00e6-4866-bf01-2027cb75e94b.client4150/metrics >>>> item total avg_lat(us) min_lat(us) max_lat(us) stdev(us) >>>> ----------------------------------------------------------------------------------- >>>> read 2312 9000 1000 100000 607.4 >>>> write 21777 925000 2000 44551000 29700.3 >>>> metadata 6 4179000 1000 21414000 19590.8 >>>> >>>> item total miss hit >>>> ------------------------------------------------- >>>> d_lease 2 0 11 >>>> caps 2 14 398418 >>>> >>>> >>>> >>>> Xiubo Li (4): >>>> ceph: switch to DIV64_U64_ROUND_CLOSEST to support 32-bit arches >>>> ceph: add min/max latency support for read/write/metadata metrics >>>> ceph: move the metric helpers into one separate file >>>> ceph: add standard deviation support for read/write/metadata perf >>>> metric >>>> >>>> fs/ceph/Makefile | 2 +- >>>> fs/ceph/debugfs.c | 89 ++++++++++++++++++------ >>>> fs/ceph/mds_client.c | 83 +--------------------- >>>> fs/ceph/metric.c | 190 +++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> fs/ceph/metric.h | 79 +++++++++++---------- >>>> 5 files changed, 297 insertions(+), 146 deletions(-) >>>> create mode 100644 fs/ceph/metric.c >>> Hi Xiubo, >>> >>> I think these additions need to be merged with your previous series, >>> so that the history is clean. Ideally the whole thing would start with >>> a single patch adding all of the metrics infrastructure to metric.[ch], >>> followed by patches introducing new metrics and ceph_update_*() calls. >>> >>> Related metrics and ceph_update_*() calls should be added together. >>> No point in splitting read and write OSD latency in two patches as they >>> touch the same functions in addr.c and file.c. >> Hi Ilya, >> >> Yeah, it makes sense and I will merge all the related patch series about >> the metric and post it again. >> > Sounds good. I've gone ahead and dropped all of the metrics patches from > the "testing" branch for now. Please resend the whole series and I'll > re-merge them. Sure, thanks Jeff. BRs > Thanks,
From: Xiubo Li <xiubli@redhat.com> Changed in V4: - fix the 32-bit arches div errors by using DIV64_U64_ROUND_CLOSEST instead. [1/4] - rebase and combine the stdev patch series [3/4][4/4] - remove the sum latency showing, which makes no sense for debugging, if it is really needed in some case then just do (avg * total) in userland. [4/4] - switch {read/write/metadata}_latency_sum to atomic type since it will be readed very time when updating the latencies to calculate the stdev. [4/4] Changed in V2: - switch spin lock to cmpxchg [1/4] Changed in V3: - add the __update_min/max_latency helpers [1/4] # cat /sys/kernel/debug/ceph/0f923fe5-00e6-4866-bf01-2027cb75e94b.client4150/metrics item total avg_lat(us) min_lat(us) max_lat(us) stdev(us) ----------------------------------------------------------------------------------- read 2312 9000 1000 100000 607.4 write 21777 925000 2000 44551000 29700.3 metadata 6 4179000 1000 21414000 19590.8 item total miss hit ------------------------------------------------- d_lease 2 0 11 caps 2 14 398418 Xiubo Li (4): ceph: switch to DIV64_U64_ROUND_CLOSEST to support 32-bit arches ceph: add min/max latency support for read/write/metadata metrics ceph: move the metric helpers into one separate file ceph: add standard deviation support for read/write/metadata perf metric fs/ceph/Makefile | 2 +- fs/ceph/debugfs.c | 89 ++++++++++++++++++------ fs/ceph/mds_client.c | 83 +--------------------- fs/ceph/metric.c | 190 +++++++++++++++++++++++++++++++++++++++++++++++++++ fs/ceph/metric.h | 79 +++++++++++---------- 5 files changed, 297 insertions(+), 146 deletions(-) create mode 100644 fs/ceph/metric.c