[PATCH v2 0/2] ceph: metrics for remote object copies

[PATCH v2 0/2] ceph: metrics for remote object copies

[Luís Henriques]

Hi!

Here's v2 of this patchset.  The differences from v1:

  * Instead of changing ceph_osdc_copy_from() in libceph.ko to return an
    osd request, move that function into the cephfs code instead.

Other than that, the 2nd patch is quite similar to the one from v1: it
effectively hooks the 'copyfrom' metrics infrastructure.

Luís Henriques (2):
  ceph: libceph: move ceph_osdc_copy_from() into cephfs code
  ceph: add a new metric to keep track of remote object copies

 fs/ceph/debugfs.c               |  3 +-
 fs/ceph/file.c                  | 78 ++++++++++++++++++++++++++++-----
 fs/ceph/metric.h                |  8 ++++
 include/linux/ceph/osd_client.h | 19 ++++----
 net/ceph/osd_client.c           | 60 ++++---------------------
 5 files changed, 94 insertions(+), 74 deletions(-)

[PATCH v2 1/2] ceph: libceph: move ceph_osdc_copy_from() into cephfs code

[Luís Henriques]

This patch moves ceph_osdc_copy_from() function out of libceph code into
cephfs.  There are no other users for this function, and there is the need
(in another patch) to access internal ceph_osd_request struct members.
Thus, instead of changing ceph_osdc_copy_from() to return the request,
simply move it where it is needed.

Signed-off-by: Luís Henriques <lhenriques@suse.de>
---
 fs/ceph/file.c                  | 74 ++++++++++++++++++++++++++++-----
 include/linux/ceph/osd_client.h | 19 ++++-----
 net/ceph/osd_client.c           | 60 ++++----------------------
 3 files changed, 80 insertions(+), 73 deletions(-)

diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 6005b430f6f7..6c77f203e7b5 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -2211,6 +2211,54 @@ static int is_file_size_ok(struct inode *src_inode, struct inode *dst_inode,
 	return 0;
 }
 
+static struct ceph_osd_request *
+ceph_alloc_copyfrom_request(struct ceph_osd_client *osdc,
+			    u64 src_snapid,
+			    struct ceph_object_id *src_oid,
+			    struct ceph_object_locator *src_oloc,
+			    struct ceph_object_id *dst_oid,
+			    struct ceph_object_locator *dst_oloc,
+			    u32 truncate_seq, u64 truncate_size)
+{
+	struct ceph_osd_request *req;
+	int ret;
+	u32 src_fadvise_flags =
+		CEPH_OSD_OP_FLAG_FADVISE_SEQUENTIAL |
+		CEPH_OSD_OP_FLAG_FADVISE_NOCACHE;
+	u32 dst_fadvise_flags =
+		CEPH_OSD_OP_FLAG_FADVISE_SEQUENTIAL |
+		CEPH_OSD_OP_FLAG_FADVISE_DONTNEED;
+
+	req = ceph_osdc_alloc_request(osdc, NULL, 1, false, GFP_KERNEL);
+	if (!req)
+		return ERR_PTR(-ENOMEM);
+
+	req->r_flags = CEPH_OSD_FLAG_WRITE;
+
+	ceph_oloc_copy(&req->r_t.base_oloc, dst_oloc);
+	ceph_oid_copy(&req->r_t.base_oid, dst_oid);
+
+	ret = osd_req_op_copy_from_init(req, src_snapid, 0,
+					src_oid, src_oloc,
+					src_fadvise_flags,
+					dst_fadvise_flags,
+					truncate_seq,
+					truncate_size,
+					CEPH_OSD_COPY_FROM_FLAG_TRUNCATE_SEQ);
+	if (ret)
+		goto out;
+
+	ret = ceph_osdc_alloc_messages(req, GFP_KERNEL);
+	if (ret)
+		goto out;
+
+	return req;
+
+out:
+	ceph_osdc_put_request(req);
+	return ERR_PTR(ret);
+}
+
 static ssize_t ceph_do_objects_copy(struct ceph_inode_info *src_ci, u64 *src_off,
 				    struct ceph_inode_info *dst_ci, u64 *dst_off,
 				    struct ceph_fs_client *fsc,
@@ -2218,6 +2266,8 @@ 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_client *osdc;
+	struct ceph_osd_request *req;
 	size_t bytes = 0;
 	u64 src_objnum, src_objoff, dst_objnum, dst_objoff;
 	u32 src_objlen, dst_objlen;
@@ -2228,6 +2278,7 @@ static ssize_t ceph_do_objects_copy(struct ceph_inode_info *src_ci, u64 *src_off
 	src_oloc.pool_ns = ceph_try_get_string(src_ci->i_layout.pool_ns);
 	dst_oloc.pool = dst_ci->i_layout.pool_id;
 	dst_oloc.pool_ns = ceph_try_get_string(dst_ci->i_layout.pool_ns);
+	osdc = &fsc->client->osdc;
 
 	while (len >= object_size) {
 		ceph_calc_file_object_mapping(&src_ci->i_layout, *src_off,
@@ -2243,17 +2294,18 @@ 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,
-					  src_ci->i_vino.snap, 0,
-					  &src_oid, &src_oloc,
-					  CEPH_OSD_OP_FLAG_FADVISE_SEQUENTIAL |
-					  CEPH_OSD_OP_FLAG_FADVISE_NOCACHE,
-					  &dst_oid, &dst_oloc,
-					  CEPH_OSD_OP_FLAG_FADVISE_SEQUENTIAL |
-					  CEPH_OSD_OP_FLAG_FADVISE_DONTNEED,
-					  dst_ci->i_truncate_seq,
-					  dst_ci->i_truncate_size,
-					  CEPH_OSD_COPY_FROM_FLAG_TRUNCATE_SEQ);
+		req = ceph_alloc_copyfrom_request(osdc, src_ci->i_vino.snap,
+						  &src_oid, &src_oloc,
+						  &dst_oid, &dst_oloc,
+						  dst_ci->i_truncate_seq,
+						  dst_ci->i_truncate_size);
+		if (IS_ERR(req))
+			ret = PTR_ERR(req);
+		else {
+			ceph_osdc_start_request(osdc, req, false);
+			ret = ceph_osdc_wait_request(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..3431011f364d 100644
--- a/include/linux/ceph/osd_client.h
+++ b/include/linux/ceph/osd_client.h
@@ -475,6 +475,14 @@ extern void osd_req_op_alloc_hint_init(struct ceph_osd_request *osd_req,
 				       u64 expected_object_size,
 				       u64 expected_write_size,
 				       u32 flags);
+extern int osd_req_op_copy_from_init(struct ceph_osd_request *req,
+				     u64 src_snapid, u64 src_version,
+				     struct ceph_object_id *src_oid,
+				     struct ceph_object_locator *src_oloc,
+				     u32 src_fadvise_flags,
+				     u32 dst_fadvise_flags,
+				     u32 truncate_seq, u64 truncate_size,
+				     u8 copy_from_flags);
 
 extern struct ceph_osd_request *ceph_osdc_alloc_request(struct ceph_osd_client *osdc,
 					       struct ceph_snap_context *snapc,
@@ -515,17 +523,6 @@ 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);
-
 /* watch/notify */
 struct ceph_osd_linger_request *
 ceph_osdc_watch(struct ceph_osd_client *osdc,
diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
index ff8624a7c964..1c5815530e0d 100644
--- a/net/ceph/osd_client.c
+++ b/net/ceph/osd_client.c
@@ -5310,14 +5310,14 @@ void ceph_osdc_stop(struct ceph_osd_client *osdc)
 	ceph_msgpool_destroy(&osdc->msgpool_op_reply);
 }
 
-static int osd_req_op_copy_from_init(struct ceph_osd_request *req,
-				     u64 src_snapid, u64 src_version,
-				     struct ceph_object_id *src_oid,
-				     struct ceph_object_locator *src_oloc,
-				     u32 src_fadvise_flags,
-				     u32 dst_fadvise_flags,
-				     u32 truncate_seq, u64 truncate_size,
-				     u8 copy_from_flags)
+int osd_req_op_copy_from_init(struct ceph_osd_request *req,
+			      u64 src_snapid, u64 src_version,
+			      struct ceph_object_id *src_oid,
+			      struct ceph_object_locator *src_oloc,
+			      u32 src_fadvise_flags,
+			      u32 dst_fadvise_flags,
+			      u32 truncate_seq, u64 truncate_size,
+			      u8 copy_from_flags)
 {
 	struct ceph_osd_req_op *op;
 	struct page **pages;
@@ -5346,49 +5346,7 @@ static int osd_req_op_copy_from_init(struct ceph_osd_request *req,
 				 op->indata_len, 0, false, true);
 	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 *req;
-	int ret;
-
-	req = ceph_osdc_alloc_request(osdc, NULL, 1, false, GFP_KERNEL);
-	if (!req)
-		return -ENOMEM;
-
-	req->r_flags = CEPH_OSD_FLAG_WRITE;
-
-	ceph_oloc_copy(&req->r_t.base_oloc, dst_oloc);
-	ceph_oid_copy(&req->r_t.base_oid, dst_oid);
-
-	ret = osd_req_op_copy_from_init(req, src_snapid, src_version, src_oid,
-					src_oloc, src_fadvise_flags,
-					dst_fadvise_flags, truncate_seq,
-					truncate_size, copy_from_flags);
-	if (ret)
-		goto out;
-
-	ret = ceph_osdc_alloc_messages(req, GFP_KERNEL);
-	if (ret)
-		goto out;
-
-	ceph_osdc_start_request(osdc, req, false);
-	ret = ceph_osdc_wait_request(osdc, req);
-
-out:
-	ceph_osdc_put_request(req);
-	return ret;
-}
-EXPORT_SYMBOL(ceph_osdc_copy_from);
+EXPORT_SYMBOL(osd_req_op_copy_from_init);
 
 int __init ceph_osdc_setup(void)
 {

Re: [PATCH v2 1/2] ceph: libceph: move ceph_osdc_copy_from() into cephfs code

[Ilya Dryomov]

On Thu, Nov 4, 2021 at 1:31 PM Luís Henriques <lhenriques@suse.de> wrote:
>
> This patch moves ceph_osdc_copy_from() function out of libceph code into
> cephfs.  There are no other users for this function, and there is the need
> (in another patch) to access internal ceph_osd_request struct members.
> Thus, instead of changing ceph_osdc_copy_from() to return the request,
> simply move it where it is needed.

Hi Luis,

ceph_alloc_copyfrom_request() does exactly that -- returns the request.
I have dropped this sentence from the changelog but wanted to check if
you meant to keep the change instead of just moving and renaming.

Thanks,

                Ilya

Re: [PATCH v2 1/2] ceph: libceph: move ceph_osdc_copy_from() into cephfs code

[Luís Henriques]

Hi Ilya,

On Mon, Nov 08, 2021 at 04:46:40AM +0100, Ilya Dryomov wrote:
> On Thu, Nov 4, 2021 at 1:31 PM Luís Henriques <lhenriques@suse.de> wrote:
> >
> > This patch moves ceph_osdc_copy_from() function out of libceph code into
> > cephfs.  There are no other users for this function, and there is the need
> > (in another patch) to access internal ceph_osd_request struct members.
> > Thus, instead of changing ceph_osdc_copy_from() to return the request,
> > simply move it where it is needed.
> 
> Hi Luis,
> 
> ceph_alloc_copyfrom_request() does exactly that -- returns the request.
> I have dropped this sentence from the changelog but wanted to check if
> you meant to keep the change instead of just moving and renaming.

Dropping that sentence seems the right thing to do.  I've just checked the
'testing' branch and the changelog looks good to me.  Thanks!

Cheers,
--
Luís

[PATCH v2 2/2] ceph: add a new metric to keep track of remote object copies

[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 6c77f203e7b5..220a41831b46 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -2304,6 +2304,10 @@ static ssize_t ceph_do_objects_copy(struct ceph_inode_info *src_ci, u64 *src_off
 		else {
 			ceph_osdc_start_request(osdc, req, false);
 			ret = ceph_osdc_wait_request(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 */

Re: [PATCH v2 0/2] ceph: metrics for remote object copies

[Jeff Layton]

On Thu, 2021-11-04 at 12:31 +0000, Luís Henriques wrote:
> Hi!
> 
> Here's v2 of this patchset.  The differences from v1:
> 
>   * Instead of changing ceph_osdc_copy_from() in libceph.ko to return an
>     osd request, move that function into the cephfs code instead.
> 
> Other than that, the 2nd patch is quite similar to the one from v1: it
> effectively hooks the 'copyfrom' metrics infrastructure.
> 
> Luís Henriques (2):
>   ceph: libceph: move ceph_osdc_copy_from() into cephfs code
>   ceph: add a new metric to keep track of remote object copies
> 
>  fs/ceph/debugfs.c               |  3 +-
>  fs/ceph/file.c                  | 78 ++++++++++++++++++++++++++++-----
>  fs/ceph/metric.h                |  8 ++++
>  include/linux/ceph/osd_client.h | 19 ++++----
>  net/ceph/osd_client.c           | 60 ++++---------------------
>  5 files changed, 94 insertions(+), 74 deletions(-)
> 

Looks good. Thanks, Luis. Merged into testing branch.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 0/2] ceph: metrics for remote object copies

[Luís Henriques]

On Thu, Nov 04, 2021 at 11:09:32AM -0400, Jeff Layton wrote:
> On Thu, 2021-11-04 at 12:31 +0000, Luís Henriques wrote:
> > Hi!
> > 
> > Here's v2 of this patchset.  The differences from v1:
> > 
> >   * Instead of changing ceph_osdc_copy_from() in libceph.ko to return an
> >     osd request, move that function into the cephfs code instead.
> > 
> > Other than that, the 2nd patch is quite similar to the one from v1: it
> > effectively hooks the 'copyfrom' metrics infrastructure.
> > 
> > Luís Henriques (2):
> >   ceph: libceph: move ceph_osdc_copy_from() into cephfs code
> >   ceph: add a new metric to keep track of remote object copies
> > 
> >  fs/ceph/debugfs.c               |  3 +-
> >  fs/ceph/file.c                  | 78 ++++++++++++++++++++++++++++-----
> >  fs/ceph/metric.h                |  8 ++++
> >  include/linux/ceph/osd_client.h | 19 ++++----
> >  net/ceph/osd_client.c           | 60 ++++---------------------
> >  5 files changed, 94 insertions(+), 74 deletions(-)
> > 
> 
> Looks good. Thanks, Luis. Merged into testing branch.

Awesome, thanks.  I'll wait until this is merged into mainline (which will
take a while, of course) before I push changes to the fstests.  Adding
further checks to the tests that use remote copies was the drive for these
new metrics, and I've already some patches for doing that.  They'll need
some cleanup but it doesn't make sense to push them before this is
available upstream.

Cheers,
--
Luís