* [PATCH 0/2] ceph: metrics for remote object copies @ 2021-11-03 11:24 Luís Henriques 2021-11-03 11:24 ` [PATCH 1/2] libceph: have ceph_osdc_copy_from() return the osd request Luís Henriques 2021-11-03 11:24 ` [PATCH 2/2] ceph: add a new metric to keep track of remote object copies Luís Henriques 0 siblings, 2 replies; 5+ messages in thread From: Luís Henriques @ 2021-11-03 11:24 UTC (permalink / raw) To: Jeff Layton, Ilya Dryomov, Xiubo Li Cc: Patrick Donnelly, ceph-devel, linux-kernel, Luís Henriques Following this email, I'm sending two patches that add support for a an extra metric in the cephfs metrics infrastructure. The 1st patch modifies libceph so that ceph_osdc_copy_from() returns an OSD request and makes it responsibility of the caller to do the wait (and release the request). This is required so that the callers (currently only the copy_file_range() syscall code) can access the request latency timestamps. The 2nd patch effectively adds support for the 'copyfrom' metrics. Luís Henriques (2): libceph: have ceph_osdc_copy_from() return the osd request ceph: add a new metric to keep track of remote object copies fs/ceph/debugfs.c | 3 ++- fs/ceph/file.c | 13 ++++++++++++- fs/ceph/metric.h | 8 ++++++++ include/linux/ceph/osd_client.h | 21 +++++++++++---------- net/ceph/osd_client.c | 27 ++++++++++++++------------- 5 files changed, 47 insertions(+), 25 deletions(-) ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] libceph: have ceph_osdc_copy_from() return the osd request 2021-11-03 11:24 [PATCH 0/2] ceph: metrics for remote object copies Luís Henriques @ 2021-11-03 11:24 ` Luís Henriques 2021-11-03 13:06 ` Jeff Layton 2021-11-03 11:24 ` [PATCH 2/2] ceph: add a new metric to keep track of remote object copies Luís Henriques 1 sibling, 1 reply; 5+ messages in thread From: Luís Henriques @ 2021-11-03 11:24 UTC (permalink / raw) To: Jeff Layton, Ilya Dryomov, Xiubo Li Cc: Patrick Donnelly, ceph-devel, linux-kernel, Luís Henriques This patch modifies the behaviour of ceph_osdc_copy_from() function so that it will create the osd request and send it but won't block waiting for the result. It is now the responsibility of the callers (currently only ceph_do_objects_copy()) to do the wait and release the request. Signed-off-by: Luís Henriques <lhenriques@suse.de> --- fs/ceph/file.c | 9 ++++++++- include/linux/ceph/osd_client.h | 21 +++++++++++---------- net/ceph/osd_client.c | 27 ++++++++++++++------------- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/fs/ceph/file.c b/fs/ceph/file.c index 6005b430f6f7..a39703b8ef99 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -2218,6 +2218,7 @@ static ssize_t ceph_do_objects_copy(struct ceph_inode_info *src_ci, u64 *src_off { struct ceph_object_locator src_oloc, dst_oloc; struct ceph_object_id src_oid, dst_oid; + struct ceph_osd_request *req; size_t bytes = 0; u64 src_objnum, src_objoff, dst_objnum, dst_objoff; u32 src_objlen, dst_objlen; @@ -2243,7 +2244,7 @@ static ssize_t ceph_do_objects_copy(struct ceph_inode_info *src_ci, u64 *src_off ceph_oid_printf(&dst_oid, "%llx.%08llx", dst_ci->i_vino.ino, dst_objnum); /* Do an object remote copy */ - ret = ceph_osdc_copy_from(&fsc->client->osdc, + req = ceph_osdc_copy_from(&fsc->client->osdc, src_ci->i_vino.snap, 0, &src_oid, &src_oloc, CEPH_OSD_OP_FLAG_FADVISE_SEQUENTIAL | @@ -2254,6 +2255,12 @@ static ssize_t ceph_do_objects_copy(struct ceph_inode_info *src_ci, u64 *src_off dst_ci->i_truncate_seq, dst_ci->i_truncate_size, CEPH_OSD_COPY_FROM_FLAG_TRUNCATE_SEQ); + if (IS_ERR(req)) + ret = PTR_ERR(req); + else { + ret = ceph_osdc_wait_request(&fsc->client->osdc, req); + ceph_osdc_put_request(req); + } if (ret) { if (ret == -EOPNOTSUPP) { fsc->have_copy_from2 = false; diff --git a/include/linux/ceph/osd_client.h b/include/linux/ceph/osd_client.h index 83fa08a06507..74d590cd29c9 100644 --- a/include/linux/ceph/osd_client.h +++ b/include/linux/ceph/osd_client.h @@ -515,16 +515,17 @@ int ceph_osdc_call(struct ceph_osd_client *osdc, struct page *req_page, size_t req_len, struct page **resp_pages, size_t *resp_len); -int ceph_osdc_copy_from(struct ceph_osd_client *osdc, - u64 src_snapid, u64 src_version, - struct ceph_object_id *src_oid, - struct ceph_object_locator *src_oloc, - u32 src_fadvise_flags, - struct ceph_object_id *dst_oid, - struct ceph_object_locator *dst_oloc, - u32 dst_fadvise_flags, - u32 truncate_seq, u64 truncate_size, - u8 copy_from_flags); +struct ceph_osd_request * +ceph_osdc_copy_from(struct ceph_osd_client *osdc, + u64 src_snapid, u64 src_version, + struct ceph_object_id *src_oid, + struct ceph_object_locator *src_oloc, + u32 src_fadvise_flags, + struct ceph_object_id *dst_oid, + struct ceph_object_locator *dst_oloc, + u32 dst_fadvise_flags, + u32 truncate_seq, u64 truncate_size, + u8 copy_from_flags); /* watch/notify */ struct ceph_osd_linger_request * diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c index ff8624a7c964..78384b431748 100644 --- a/net/ceph/osd_client.c +++ b/net/ceph/osd_client.c @@ -5347,23 +5347,24 @@ static int osd_req_op_copy_from_init(struct ceph_osd_request *req, return 0; } -int ceph_osdc_copy_from(struct ceph_osd_client *osdc, - u64 src_snapid, u64 src_version, - struct ceph_object_id *src_oid, - struct ceph_object_locator *src_oloc, - u32 src_fadvise_flags, - struct ceph_object_id *dst_oid, - struct ceph_object_locator *dst_oloc, - u32 dst_fadvise_flags, - u32 truncate_seq, u64 truncate_size, - u8 copy_from_flags) +struct ceph_osd_request * +ceph_osdc_copy_from(struct ceph_osd_client *osdc, + u64 src_snapid, u64 src_version, + struct ceph_object_id *src_oid, + struct ceph_object_locator *src_oloc, + u32 src_fadvise_flags, + struct ceph_object_id *dst_oid, + struct ceph_object_locator *dst_oloc, + u32 dst_fadvise_flags, + u32 truncate_seq, u64 truncate_size, + u8 copy_from_flags) { struct ceph_osd_request *req; int ret; req = ceph_osdc_alloc_request(osdc, NULL, 1, false, GFP_KERNEL); if (!req) - return -ENOMEM; + return ERR_PTR(-ENOMEM); req->r_flags = CEPH_OSD_FLAG_WRITE; @@ -5382,11 +5383,11 @@ int ceph_osdc_copy_from(struct ceph_osd_client *osdc, goto out; ceph_osdc_start_request(osdc, req, false); - ret = ceph_osdc_wait_request(osdc, req); + return req; out: ceph_osdc_put_request(req); - return ret; + return ERR_PTR(ret); } EXPORT_SYMBOL(ceph_osdc_copy_from); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] libceph: have ceph_osdc_copy_from() return the osd request 2021-11-03 11:24 ` [PATCH 1/2] libceph: have ceph_osdc_copy_from() return the osd request Luís Henriques @ 2021-11-03 13:06 ` Jeff Layton 2021-11-03 14:32 ` Luís Henriques 0 siblings, 1 reply; 5+ messages in thread From: Jeff Layton @ 2021-11-03 13:06 UTC (permalink / raw) To: Luís Henriques, Ilya Dryomov, Xiubo Li Cc: Patrick Donnelly, ceph-devel, linux-kernel On Wed, 2021-11-03 at 11:24 +0000, Luís Henriques wrote: > This patch modifies the behaviour of ceph_osdc_copy_from() function so > that it will create the osd request and send it but won't block waiting > for the result. It is now the responsibility of the callers (currently > only ceph_do_objects_copy()) to do the wait and release the request. > > Signed-off-by: Luís Henriques <lhenriques@suse.de> > --- > fs/ceph/file.c | 9 ++++++++- > include/linux/ceph/osd_client.h | 21 +++++++++++---------- > net/ceph/osd_client.c | 27 ++++++++++++++------------- > 3 files changed, 33 insertions(+), 24 deletions(-) > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c > index 6005b430f6f7..a39703b8ef99 100644 > --- a/fs/ceph/file.c > +++ b/fs/ceph/file.c > @@ -2218,6 +2218,7 @@ static ssize_t ceph_do_objects_copy(struct ceph_inode_info *src_ci, u64 *src_off > { > struct ceph_object_locator src_oloc, dst_oloc; > struct ceph_object_id src_oid, dst_oid; > + struct ceph_osd_request *req; > size_t bytes = 0; > u64 src_objnum, src_objoff, dst_objnum, dst_objoff; > u32 src_objlen, dst_objlen; > @@ -2243,7 +2244,7 @@ static ssize_t ceph_do_objects_copy(struct ceph_inode_info *src_ci, u64 *src_off > ceph_oid_printf(&dst_oid, "%llx.%08llx", > dst_ci->i_vino.ino, dst_objnum); > /* Do an object remote copy */ > - ret = ceph_osdc_copy_from(&fsc->client->osdc, > + req = ceph_osdc_copy_from(&fsc->client->osdc, > src_ci->i_vino.snap, 0, > &src_oid, &src_oloc, > CEPH_OSD_OP_FLAG_FADVISE_SEQUENTIAL | > @@ -2254,6 +2255,12 @@ static ssize_t ceph_do_objects_copy(struct ceph_inode_info *src_ci, u64 *src_off > dst_ci->i_truncate_seq, > dst_ci->i_truncate_size, > CEPH_OSD_COPY_FROM_FLAG_TRUNCATE_SEQ); > + if (IS_ERR(req)) > + ret = PTR_ERR(req); > + else { > + ret = ceph_osdc_wait_request(&fsc->client->osdc, req); > + ceph_osdc_put_request(req); > + } > if (ret) { > if (ret == -EOPNOTSUPP) { > fsc->have_copy_from2 = false; > diff --git a/include/linux/ceph/osd_client.h b/include/linux/ceph/osd_client.h > index 83fa08a06507..74d590cd29c9 100644 > --- a/include/linux/ceph/osd_client.h > +++ b/include/linux/ceph/osd_client.h > @@ -515,16 +515,17 @@ int ceph_osdc_call(struct ceph_osd_client *osdc, > struct page *req_page, size_t req_len, > struct page **resp_pages, size_t *resp_len); > > -int ceph_osdc_copy_from(struct ceph_osd_client *osdc, > - u64 src_snapid, u64 src_version, > - struct ceph_object_id *src_oid, > - struct ceph_object_locator *src_oloc, > - u32 src_fadvise_flags, > - struct ceph_object_id *dst_oid, > - struct ceph_object_locator *dst_oloc, > - u32 dst_fadvise_flags, > - u32 truncate_seq, u64 truncate_size, > - u8 copy_from_flags); > +struct ceph_osd_request * > +ceph_osdc_copy_from(struct ceph_osd_client *osdc, > + u64 src_snapid, u64 src_version, > + struct ceph_object_id *src_oid, > + struct ceph_object_locator *src_oloc, > + u32 src_fadvise_flags, > + struct ceph_object_id *dst_oid, > + struct ceph_object_locator *dst_oloc, > + u32 dst_fadvise_flags, > + u32 truncate_seq, u64 truncate_size, > + u8 copy_from_flags); > > /* watch/notify */ > struct ceph_osd_linger_request * > diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c > index ff8624a7c964..78384b431748 100644 > --- a/net/ceph/osd_client.c > +++ b/net/ceph/osd_client.c > @@ -5347,23 +5347,24 @@ static int osd_req_op_copy_from_init(struct ceph_osd_request *req, > return 0; > } > > -int ceph_osdc_copy_from(struct ceph_osd_client *osdc, > - u64 src_snapid, u64 src_version, > - struct ceph_object_id *src_oid, > - struct ceph_object_locator *src_oloc, > - u32 src_fadvise_flags, > - struct ceph_object_id *dst_oid, > - struct ceph_object_locator *dst_oloc, > - u32 dst_fadvise_flags, > - u32 truncate_seq, u64 truncate_size, > - u8 copy_from_flags) > +struct ceph_osd_request * > +ceph_osdc_copy_from(struct ceph_osd_client *osdc, > + u64 src_snapid, u64 src_version, > + struct ceph_object_id *src_oid, > + struct ceph_object_locator *src_oloc, > + u32 src_fadvise_flags, > + struct ceph_object_id *dst_oid, > + struct ceph_object_locator *dst_oloc, > + u32 dst_fadvise_flags, > + u32 truncate_seq, u64 truncate_size, > + u8 copy_from_flags) > { > struct ceph_osd_request *req; > int ret; > > req = ceph_osdc_alloc_request(osdc, NULL, 1, false, GFP_KERNEL); > if (!req) > - return -ENOMEM; > + return ERR_PTR(-ENOMEM); > > req->r_flags = CEPH_OSD_FLAG_WRITE; > > @@ -5382,11 +5383,11 @@ int ceph_osdc_copy_from(struct ceph_osd_client *osdc, > goto out; > > ceph_osdc_start_request(osdc, req, false); > - ret = ceph_osdc_wait_request(osdc, req); > + return req; > I don't really understand why this function is part of net/ceph. It's odd in that it basically allocates, initializes and starts the request and then passes back the pointer to it. That doesn't really follow the pattern of the other OSD READ/WRITE ops, where we basically set them up and start them more directly from the fs/ceph code. I think it'd make more sense to just get rid of ceph_osdc_copy_from altogether, export the osd_req_op_copy_from_init symbol and open-code the whole thing in ceph_do_objects_copy. This isn't otherwise called from rbd or anything else so I don't see the benefit of keeping this function as part of libceph. > out: > ceph_osdc_put_request(req); > - return ret; > + return ERR_PTR(ret); > } > EXPORT_SYMBOL(ceph_osdc_copy_from); > -- Jeff Layton <jlayton@kernel.org> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] libceph: have ceph_osdc_copy_from() return the osd request 2021-11-03 13:06 ` Jeff Layton @ 2021-11-03 14:32 ` Luís Henriques 0 siblings, 0 replies; 5+ messages in thread From: Luís Henriques @ 2021-11-03 14:32 UTC (permalink / raw) To: Jeff Layton Cc: Ilya Dryomov, Xiubo Li, Patrick Donnelly, ceph-devel, linux-kernel On Wed, Nov 03, 2021 at 09:06:31AM -0400, Jeff Layton wrote: <...> > > diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c > > index ff8624a7c964..78384b431748 100644 > > --- a/net/ceph/osd_client.c > > +++ b/net/ceph/osd_client.c > > @@ -5347,23 +5347,24 @@ static int osd_req_op_copy_from_init(struct ceph_osd_request *req, > > return 0; > > } > > > > -int ceph_osdc_copy_from(struct ceph_osd_client *osdc, > > - u64 src_snapid, u64 src_version, > > - struct ceph_object_id *src_oid, > > - struct ceph_object_locator *src_oloc, > > - u32 src_fadvise_flags, > > - struct ceph_object_id *dst_oid, > > - struct ceph_object_locator *dst_oloc, > > - u32 dst_fadvise_flags, > > - u32 truncate_seq, u64 truncate_size, > > - u8 copy_from_flags) > > +struct ceph_osd_request * > > +ceph_osdc_copy_from(struct ceph_osd_client *osdc, > > + u64 src_snapid, u64 src_version, > > + struct ceph_object_id *src_oid, > > + struct ceph_object_locator *src_oloc, > > + u32 src_fadvise_flags, > > + struct ceph_object_id *dst_oid, > > + struct ceph_object_locator *dst_oloc, > > + u32 dst_fadvise_flags, > > + u32 truncate_seq, u64 truncate_size, > > + u8 copy_from_flags) > > { > > struct ceph_osd_request *req; > > int ret; > > > > req = ceph_osdc_alloc_request(osdc, NULL, 1, false, GFP_KERNEL); > > if (!req) > > - return -ENOMEM; > > + return ERR_PTR(-ENOMEM); > > > > req->r_flags = CEPH_OSD_FLAG_WRITE; > > > > @@ -5382,11 +5383,11 @@ int ceph_osdc_copy_from(struct ceph_osd_client *osdc, > > goto out; > > > > ceph_osdc_start_request(osdc, req, false); > > - ret = ceph_osdc_wait_request(osdc, req); > > + return req; > > > > I don't really understand why this function is part of net/ceph. It's > odd in that it basically allocates, initializes and starts the request > and then passes back the pointer to it. That doesn't really follow the > pattern of the other OSD READ/WRITE ops, where we basically set them up > and start them more directly from the fs/ceph code. > > I think it'd make more sense to just get rid of ceph_osdc_copy_from > altogether, export the osd_req_op_copy_from_init symbol and open-code > the whole thing in ceph_do_objects_copy. This isn't otherwise called > from rbd or anything else so I don't see the benefit of keeping this > function as part of libceph. At the time ceph_osdc_copy_from() was implemented, it looked like something that would make sense to keep on the osd client code, and eventually useful for rbd too. But since that didn't happen, this patch shows that it makes sense to just move the code. So, yeah, sure. I'll do that. Thanks. Cheers, -- Luís ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] ceph: add a new metric to keep track of remote object copies 2021-11-03 11:24 [PATCH 0/2] ceph: metrics for remote object copies Luís Henriques 2021-11-03 11:24 ` [PATCH 1/2] libceph: have ceph_osdc_copy_from() return the osd request Luís Henriques @ 2021-11-03 11:24 ` Luís Henriques 1 sibling, 0 replies; 5+ messages in thread From: Luís Henriques @ 2021-11-03 11:24 UTC (permalink / raw) To: Jeff Layton, Ilya Dryomov, Xiubo Li Cc: Patrick Donnelly, ceph-devel, linux-kernel, Luís Henriques This patch adds latency and size metrics for remote object copies operations ("copyfrom"). For now, these metrics will be available on the client only, they won't be sent to the MDS. Cc: Patrick Donnelly <pdonnell@redhat.com> Signed-off-by: Luís Henriques <lhenriques@suse.de> --- fs/ceph/debugfs.c | 3 ++- fs/ceph/file.c | 4 ++++ fs/ceph/metric.h | 8 ++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c index e04ae1098431..3cf7c9c1085b 100644 --- a/fs/ceph/debugfs.c +++ b/fs/ceph/debugfs.c @@ -167,7 +167,8 @@ static int metrics_file_show(struct seq_file *s, void *p) static const char * const metric_str[] = { "read", "write", - "metadata" + "metadata", + "copyfrom" }; static int metrics_latency_show(struct seq_file *s, void *p) { diff --git a/fs/ceph/file.c b/fs/ceph/file.c index a39703b8ef99..c61d71cef55d 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -2259,6 +2259,10 @@ static ssize_t ceph_do_objects_copy(struct ceph_inode_info *src_ci, u64 *src_off ret = PTR_ERR(req); else { ret = ceph_osdc_wait_request(&fsc->client->osdc, req); + ceph_update_copyfrom_metrics(&fsc->mdsc->metric, + req->r_start_latency, + req->r_end_latency, + object_size, ret); ceph_osdc_put_request(req); } if (ret) { diff --git a/fs/ceph/metric.h b/fs/ceph/metric.h index e67fc997760b..bb45608181e7 100644 --- a/fs/ceph/metric.h +++ b/fs/ceph/metric.h @@ -129,6 +129,7 @@ enum metric_type { METRIC_READ, METRIC_WRITE, METRIC_METADATA, + METRIC_COPYFROM, METRIC_MAX }; @@ -214,4 +215,11 @@ static inline void ceph_update_metadata_metrics(struct ceph_client_metric *m, ceph_update_metrics(&m->metric[METRIC_METADATA], r_start, r_end, 0, rc); } +static inline void ceph_update_copyfrom_metrics(struct ceph_client_metric *m, + ktime_t r_start, ktime_t r_end, + unsigned int size, int rc) +{ + ceph_update_metrics(&m->metric[METRIC_COPYFROM], + r_start, r_end, size, rc); +} #endif /* _FS_CEPH_MDS_METRIC_H */ ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-03 14:32 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-11-03 11:24 [PATCH 0/2] ceph: metrics for remote object copies Luís Henriques 2021-11-03 11:24 ` [PATCH 1/2] libceph: have ceph_osdc_copy_from() return the osd request Luís Henriques 2021-11-03 13:06 ` Jeff Layton 2021-11-03 14:32 ` Luís Henriques 2021-11-03 11:24 ` [PATCH 2/2] ceph: add a new metric to keep track of remote object copies Luís Henriques
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.