libceph: parameter cleanup

This series mostly cleans up parameters used by functions in libceph, in the osd client code.

-Alex

[PATCH REPOST 1/6] libceph: pass length to ceph_osdc_build_request() [PATCH REPOST 2/6] libceph: pass length to ceph_calc_file_object_mapping() [PATCH REPOST 3/6] libceph: drop snapid in ceph_calc_raw_layout() [PATCH REPOST 4/6] libceph: drop osdc from ceph_calc_raw_layout() [PATCH REPOST 5/6] libceph: don't set flags in ceph_osdc_alloc_request() [PATCH REPOST 6/6] libceph: don't set pages or bio in ceph_osdc_alloc_request()

The len argument to ceph_osdc_build_request() is set up to be passed by address, but that function never updates its value so there's no need to do this. Tighten up the interface by passing the length directly.

---
 drivers/block/rbd.c             |    2 +-
 include/linux/ceph/osd_client.h |    2 +-
 net/ceph/osd_client.c           |    6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

index cc12154..1a241a7 100644
@@ -1172,7 +1172,7 @@ static int rbd_do_request(struct request *rq,
 				snapid, ofs, &len, &bno, osd_req, ops);
 	rbd_assert(ret == 0);
 
-	ceph_osdc_build_request(osd_req, ofs, &len, ops, snapc, &mtime);
+	ceph_osdc_build_request(osd_req, ofs, len, ops, snapc, &mtime);
 
 	if (linger_req) {
 		ceph_osdc_set_request_linger(osdc, osd_req);

b/include/linux/ceph/osd_client.h
index 61562c7..4bfb458 100644

@@ -224,7 +224,7 @@ extern struct ceph_osd_request

*ceph_osdc_alloc_request(struct ceph_osd_client *
struct bio *bio);

extern void ceph_osdc_build_request(struct ceph_osd_request *req,
- u64 off, u64 *plen,
+ u64 off, u64 len,
struct ceph_osd_req_op *src_ops, struct ceph_snap_context *snapc, struct timespec *mtime);

index 2adbfcc..cdf40c6 100644
@@ -328,7 +328,7 @@ static void osd_req_encode_op(struct

ceph_osd_request *req,
*
*/
void ceph_osdc_build_request(struct ceph_osd_request *req,
- u64 off, u64 *plen,
+ u64 off, u64 len,
struct ceph_osd_req_op *src_ops, struct ceph_snap_context *snapc, struct timespec *mtime)
@@ -382,7 +382,7 @@ void ceph_osdc_build_request(struct ceph_osd_request *req,

if (flags & CEPH_OSD_FLAG_WRITE) {
req->r_request->hdr.data_off = cpu_to_le16(off);
- req->r_request->hdr.data_len = cpu_to_le32(*plen + data_len);
+ req->r_request->hdr.data_len = cpu_to_le32(len + data_len);
} else if (data_len) {
req->r_request->hdr.data_off = 0; req->r_request->hdr.data_len = cpu_to_le32(data_len);
@@ -456,7 +456,7 @@ struct ceph_osd_request *ceph_osdc_new_request(struct ceph_osd_client *osdc,
req->r_num_pages = calc_pages_for(page_align, *plen); req->r_page_alignment = page_align;

- ceph_osdc_build_request(req, off, plen, ops,
+ ceph_osdc_build_request(req, off, *plen, ops,
snapc,
mtime);

1.7.9.5

ceph_calc_file_object_mapping() takes (among other things) a "file" offset and length, and based on the layout, determines the object number ("bno") backing the affected portion of the file's data and the offset into that object where the desired range begins. It also computes the size that should be used for the request--either the amount requested or something less if that would exceed the end of the object.

This patch changes the input length parameter in this function so it is used only for input. That is, the argument will be passed by value rather than by address, so the value provided won't get updated by the function.

The value would only get updated if the length would surpass the current object, and in that case the value it got updated to would be exactly that returned in *oxlen.

Only one of the two callers is affected by this change. Update ceph_calc_raw_layout() so it records any updated value.

---
 fs/ceph/ioctl.c             |    2 +-
 include/linux/ceph/osdmap.h |    2 +-
 net/ceph/osd_client.c       |    6 ++++--
 net/ceph/osdmap.c           |    9 ++++-----
 4 files changed, 10 insertions(+), 9 deletions(-)

index 36549a4..3b22150 100644
@@ -194,7 +194,7 @@ static long ceph_ioctl_get_dataloc(struct file

*file, void __user *arg)
return -EFAULT;

down_read(&osdc->map_sem);
- r = ceph_calc_file_object_mapping(&ci->i_layout, dl.file_offset, &len,
+ r = ceph_calc_file_object_mapping(&ci->i_layout, dl.file_offset, len,
&dl.object_no, &dl.object_offset, &olen);
if (r < 0)

index 5ea57ba..1f653e2 100644
@@ -110,7 +110,7 @@ extern void ceph_osdmap_destroy(struct ceph_osdmap

*map);

/* calculate mapping of a file extent to an object */
extern int ceph_calc_file_object_mapping(struct ceph_file_layout *layout,
- u64 off, u64 *plen,
+ u64 off, u64 len,
u64 *bno, u64 *oxoff, u64 *oxlen);

/* calculate mapping of object to a placement group */

index cdf40c6..65b7d11 100644
@@ -53,13 +53,15 @@ int ceph_calc_raw_layout(struct ceph_osd_client *osdc,
 	reqhead->snapid = cpu_to_le64(snapid);
 
 	/* object extent? */
-	r = ceph_calc_file_object_mapping(layout, off, plen, bno,
+	r = ceph_calc_file_object_mapping(layout, off, orig_len, bno,
 					  &objoff, &objlen);
 	if (r < 0)
 		return r;
-	if (*plen < orig_len)
+	if (objlen < orig_len) {
+		*plen = objlen;
 		dout(" skipping last %llu, final file extent %llu~%llu\n",
 		     orig_len - *plen, off, *plen);
+	}
 
 	if (op_has_extent(op->op)) {
 		op->extent.offset = objoff;

index de73214..9dded22 100644
@@ -1010,7 +1010,7 @@ bad:
  * pass a stride back to the caller.
  */
 int ceph_calc_file_object_mapping(struct ceph_file_layout *layout,
-				   u64 off, u64 *plen,
+				   u64 off, u64 len,
 				   u64 *ono,
 				   u64 *oxoff, u64 *oxlen)
 {

@@ -1021,7 +1021,7 @@ int ceph_calc_file_object_mapping(struct

ceph_file_layout *layout,
u32 su_per_object;
u64 t, su_offset;

- dout("mapping %llu~%llu osize %u fl_su %u\n", off, *plen,
+ dout("mapping %llu~%llu osize %u fl_su %u\n", off, len,
osize, su);
if (su == 0 || sc == 0)
goto invalid;
@@ -1054,11 +1054,10 @@ int ceph_calc_file_object_mapping(struct ceph_file_layout *layout,

/*
* Calculate the length of the extent being written to the selected
- * object. This is the minimum of the full length requested (plen) or
+ * object. This is the minimum of the full length requested (len) or
* the remainder of the current stripe being written to. */
- *oxlen = min_t(u64, *plen, su - su_offset);
- *plen = *oxlen;
+ *oxlen = min_t(u64, len, su - su_offset);

dout(" obj extent %llu~%llu\n", *oxoff, *oxlen); return 0;

1.7.9.5

A snapshot id must be provided to ceph_calc_raw_layout() even though it is not needed at all for calculating the layout.

Where the snapshot id *is* needed is when building the request message for an osd operation.

Drop the snapid parameter from ceph_calc_raw_layout() and pass that value instead in ceph_osdc_build_request().

---
 drivers/block/rbd.c             |    4 ++--
 include/linux/ceph/osd_client.h |    2 +-
 net/ceph/osd_client.c           |   14 ++++----------
 3 files changed, 7 insertions(+), 13 deletions(-)

index 1a241a7..82a1460 100644
@@ -1169,10 +1169,10 @@ static int rbd_do_request(struct request *rq,
 
 	rbd_layout_init(&osd_req->r_file_layout, rbd_dev->spec->pool_id);
 	ret = ceph_calc_raw_layout(osdc, &osd_req->r_file_layout,
-				snapid, ofs, &len, &bno, osd_req, ops);
+				ofs, &len, &bno, osd_req, ops);
 	rbd_assert(ret == 0);
 
-	ceph_osdc_build_request(osd_req, ofs, len, ops, snapc, &mtime);
+	ceph_osdc_build_request(osd_req, ofs, len, ops, snapc, snapid, &mtime);
 
 	if (linger_req) {
 		ceph_osdc_set_request_linger(osdc, osd_req);

b/include/linux/ceph/osd_client.h
index 4bfb458..0e82a0a 100644

@@ -209,7 +209,6 @@ extern void ceph_osdc_handle_map(struct

ceph_osd_client *osdc,

extern int ceph_calc_raw_layout(struct ceph_osd_client *osdc,
struct ceph_file_layout *layout,
- u64 snapid,
u64 off, u64 *plen, u64 *bno, struct ceph_osd_request *req, struct ceph_osd_req_op *op);
@@ -227,6 +226,7 @@ extern void ceph_osdc_build_request(struct ceph_osd_request *req,
u64 off, u64 len, struct ceph_osd_req_op *src_ops, struct ceph_snap_context *snapc,
+ u64 snap_id,
struct timespec *mtime);

extern struct ceph_osd_request *ceph_osdc_new_request(struct
ceph_osd_client *,

index 65b7d11..75e57de 100644
@@ -40,18 +40,14 @@ static int op_has_extent(int op)
 
 int ceph_calc_raw_layout(struct ceph_osd_client *osdc,
 			struct ceph_file_layout *layout,
-			u64 snapid,
 			u64 off, u64 *plen, u64 *bno,
 			struct ceph_osd_request *req,
 			struct ceph_osd_req_op *op)
 {
-	struct ceph_osd_request_head *reqhead = req->r_request->front.iov_base;
 	u64 orig_len = *plen;
 	u64 objoff, objlen;    /* extent in object */
 	int r;
 
-	reqhead->snapid = cpu_to_le64(snapid);
-
 	/* object extent? */
 	r = ceph_calc_file_object_mapping(layout, off, orig_len, bno,
 					  &objoff, &objlen);

@@ -113,8 +109,7 @@ static int calc_layout(struct ceph_osd_client *osdc,
 	u64 bno;
 	int r;
 
-	r = ceph_calc_raw_layout(osdc, layout, vino.snap, off,
-				 plen, &bno, req, op);
+	r = ceph_calc_raw_layout(osdc, layout, off, plen, &bno, req, op);
 	if (r < 0)
 		return r;
 

@@ -332,7 +327,7 @@ static void osd_req_encode_op(struct

ceph_osd_request *req,
void ceph_osdc_build_request(struct ceph_osd_request *req,
u64 off, u64 len,
struct ceph_osd_req_op *src_ops,
- struct ceph_snap_context *snapc,
+ struct ceph_snap_context *snapc, u64 snap_id,
struct timespec *mtime)
{
struct ceph_msg *msg = req->r_request;
@@ -347,6 +342,7 @@ void ceph_osdc_build_request(struct ceph_osd_request *req,
int i;

head = msg->front.iov_base;
+ head->snapid = cpu_to_le64(snap_id);
op = (void *)(head + 1);
p = (void *)(op + num_op);

@@ -458,9 +454,7 @@ struct ceph_osd_request *ceph_osdc_new_request(struct ceph_osd_client *osdc,
req->r_num_pages = calc_pages_for(page_align, *plen); req->r_page_alignment = page_align;

- ceph_osdc_build_request(req, off, *plen, ops,
- snapc,
- mtime);
+ ceph_osdc_build_request(req, off, *plen, ops, snapc, vino.snap, mtime);

return req;
}

1.7.9.5

The osdc parameter to ceph_calc_raw_layout() is not used, so get rid of it. Consequently, the corresponding parameter in calc_layout() becomes unused, so get rid of that as well.

---
 drivers/block/rbd.c             |    2 +-
 include/linux/ceph/osd_client.h |    3 +--
 net/ceph/osd_client.c           |   10 ++++------
 3 files changed, 6 insertions(+), 9 deletions(-)

index 82a1460..09bcf08 100644
@@ -1168,7 +1168,7 @@ static int rbd_do_request(struct request *rq,
 	osd_req->r_oid_len = strlen(osd_req->r_oid);
 
 	rbd_layout_init(&osd_req->r_file_layout, rbd_dev->spec->pool_id);
-	ret = ceph_calc_raw_layout(osdc, &osd_req->r_file_layout,
+	ret = ceph_calc_raw_layout(&osd_req->r_file_layout,
 				ofs, &len, &bno, osd_req, ops);
 	rbd_assert(ret == 0);
 

b/include/linux/ceph/osd_client.h
index 0e82a0a..fe3a6e8 100644

@@ -207,8 +207,7 @@ extern void ceph_osdc_handle_reply(struct

ceph_osd_client *osdc,
extern void ceph_osdc_handle_map(struct ceph_osd_client *osdc,
struct ceph_msg *msg);

-extern int ceph_calc_raw_layout(struct ceph_osd_client *osdc,
- struct ceph_file_layout *layout,
+extern int ceph_calc_raw_layout(struct ceph_file_layout *layout,
u64 off, u64 *plen, u64 *bno, struct ceph_osd_request *req, struct ceph_osd_req_op *op);

index 75e57de..1b55fbe 100644
@@ -38,8 +38,7 @@ static int op_has_extent(int op)
 		op == CEPH_OSD_OP_WRITE);
 }
 
-int ceph_calc_raw_layout(struct ceph_osd_client *osdc,
-			struct ceph_file_layout *layout,
+int ceph_calc_raw_layout(struct ceph_file_layout *layout,
 			u64 off, u64 *plen, u64 *bno,
 			struct ceph_osd_request *req,
 			struct ceph_osd_req_op *op)

@@ -99,8 +98,7 @@ EXPORT_SYMBOL(ceph_calc_raw_layout);
  *
  * fill osd op in request message.
  */
-static int calc_layout(struct ceph_osd_client *osdc,
-		       struct ceph_vino vino,
+static int calc_layout(struct ceph_vino vino,
 		       struct ceph_file_layout *layout,
 		       u64 off, u64 *plen,
 		       struct ceph_osd_request *req,

@@ -109,7 +107,7 @@ static int calc_layout(struct ceph_osd_client *osdc,
 	u64 bno;
 	int r;
 
-	r = ceph_calc_raw_layout(osdc, layout, off, plen, &bno, req, op);
+	r = ceph_calc_raw_layout(layout, off, plen, &bno, req, op);
 	if (r < 0)
 		return r;
 

@@ -444,7 +442,7 @@ struct ceph_osd_request

*ceph_osdc_new_request(struct ceph_osd_client *osdc,
return ERR_PTR(-ENOMEM);

/* calculate max write size */
- r = calc_layout(osdc, vino, layout, off, plen, req, ops);
+ r = calc_layout(vino, layout, off, plen, req, ops);
if (r < 0)
return ERR_PTR(r);
req->r_file_layout = *layout; /* keep a copy */

1.7.9.5

The only thing ceph_osdc_alloc_request() really does with the flags value it is passed is assign it to the newly-created osd request structure. Do that in the caller instead.

Both callers subsequently call ceph_osdc_build_request(), so have that function (instead of ceph_osdc_alloc_request()) issue a warning if a request comes through with neither the read nor write flags set.

---
 drivers/block/rbd.c             |    3 ++-
 include/linux/ceph/osd_client.h |    1 -
 net/ceph/osd_client.c           |   11 ++++-------
 3 files changed, 6 insertions(+), 9 deletions(-)

index 09bcf08..fc8bfe6 100644
@@ -1148,13 +1148,14 @@ static int rbd_do_request(struct request *rq,
 		(unsigned long long) len, coll, coll_index);
 
 	osdc = &rbd_dev->rbd_client->client->osdc;
-	osd_req = ceph_osdc_alloc_request(osdc, flags, snapc, ops,
+	osd_req = ceph_osdc_alloc_request(osdc, snapc, ops,
 					false, GFP_NOIO, pages, bio);
 	if (!osd_req) {
 		ret = -ENOMEM;
 		goto done_pages;
 	}
 
+	osd_req->r_flags = flags;
 	osd_req->r_callback = rbd_cb;
 
 	rbd_req->rq = rq;

b/include/linux/ceph/osd_client.h
index fe3a6e8..6ddda5b 100644

@@ -213,7 +213,6 @@ extern int ceph_calc_raw_layout(struct

ceph_file_layout *layout,
struct ceph_osd_req_op *op);

extern struct ceph_osd_request *ceph_osdc_alloc_request(struct
ceph_osd_client *osdc,
- int flags,
struct ceph_snap_context *snapc, struct ceph_osd_req_op *ops, bool use_mempool,

index 1b55fbe..3fe2960 100644
@@ -163,7 +163,6 @@ static int get_num_ops(struct ceph_osd_req_op *ops)
 }
 
 struct ceph_osd_request *ceph_osdc_alloc_request(struct ceph_osd_client

*osdc,
- int flags,
struct ceph_snap_context *snapc, struct ceph_osd_req_op *ops, bool use_mempool,
@@ -200,10 +199,6 @@ struct ceph_osd_request *ceph_osdc_alloc_request(struct ceph_osd_client *osdc,
INIT_LIST_HEAD(&req->r_req_lru_item); INIT_LIST_HEAD(&req->r_osd_item);

- req->r_flags = flags;
-
- WARN_ON((flags & (CEPH_OSD_FLAG_READ|CEPH_OSD_FLAG_WRITE)) == 0);
-
/* create reply message */
if (use_mempool)
msg = ceph_msgpool_get(&osdc->msgpool_op_reply, 0);
@@ -339,6 +334,8 @@ void ceph_osdc_build_request(struct ceph_osd_request *req,
u64 data_len = 0;
int i;

+ WARN_ON((flags & (CEPH_OSD_FLAG_READ|CEPH_OSD_FLAG_WRITE)) == 0);
+
head = msg->front.iov_base; head->snapid = cpu_to_le64(snap_id); op = (void *)(head + 1);
@@ -434,12 +431,12 @@ struct ceph_osd_request *ceph_osdc_new_request(struct ceph_osd_client *osdc,
} else
ops[1].op = 0;

- req = ceph_osdc_alloc_request(osdc, flags,
- snapc, ops,
+ req = ceph_osdc_alloc_request(osdc, snapc, ops,
use_mempool, GFP_NOFS, NULL, NULL);
if (!req)
return ERR_PTR(-ENOMEM);
+ req->r_flags = flags;

/* calculate max write size */
r = calc_layout(vino, layout, off, plen, req, ops);

1.7.9.5

Only one of the two callers of ceph_osdc_alloc_request() provides page or bio data for its payload. And essentially all that function was doing with those arguments was assigning them to fields in the osd request structure.

Simplify ceph_osdc_alloc_request() by having the caller take care of making those assignments

---
 drivers/block/rbd.c             |    8 ++++++--
 include/linux/ceph/osd_client.h |    4 +---
 net/ceph/osd_client.c           |   15 ++-------------
 3 files changed, 9 insertions(+), 18 deletions(-)

index fc8bfe6..6b9643a 100644
@@ -1148,14 +1148,18 @@ static int rbd_do_request(struct request *rq,
 		(unsigned long long) len, coll, coll_index);
 
 	osdc = &rbd_dev->rbd_client->client->osdc;
-	osd_req = ceph_osdc_alloc_request(osdc, snapc, ops,
-					false, GFP_NOIO, pages, bio);
+	osd_req = ceph_osdc_alloc_request(osdc, snapc, ops, false, GFP_NOIO);
 	if (!osd_req) {
 		ret = -ENOMEM;
 		goto done_pages;
 	}
 
 	osd_req->r_flags = flags;
+	osd_req->r_pages = pages;
+	if (bio) {
+		osd_req->r_bio = bio;
+		bio_get(osd_req->r_bio);
+	}
 	osd_req->r_callback = rbd_cb;
 
 	rbd_req->rq = rq;

b/include/linux/ceph/osd_client.h
index 6ddda5b..75f56d3 100644

@@ -216,9 +216,7 @@ extern struct ceph_osd_request

*ceph_osdc_alloc_request(struct ceph_osd_client *
struct ceph_snap_context *snapc, struct ceph_osd_req_op *ops, bool use_mempool,

-					       gfp_t gfp_flags,
-					       struct page **pages,
-					       struct bio *bio);
+					       gfp_t gfp_flags);

extern void ceph_osdc_build_request(struct ceph_osd_request *req,
u64 off, u64 len,

index 3fe2960..150166b 100644
@@ -166,9 +166,7 @@ struct ceph_osd_request

*ceph_osdc_alloc_request(struct ceph_osd_client *osdc,
struct ceph_snap_context *snapc, struct ceph_osd_req_op *ops, bool use_mempool,

-					       gfp_t gfp_flags,
-					       struct page **pages,
-					       struct bio *bio)
+					       gfp_t gfp_flags)

{
struct ceph_osd_request *req;
struct ceph_msg *msg;
@@ -229,13 +227,6 @@ struct ceph_osd_request *ceph_osdc_alloc_request(struct ceph_osd_client *osdc,
memset(msg->front.iov_base, 0, msg->front.iov_len);

req->r_request = msg;
- req->r_pages = pages;
-#ifdef CONFIG_BLOCK
- if (bio) {
- req->r_bio = bio;
- bio_get(req->r_bio);
- }
-#endif

return req;
}
@@ -431,9 +422,7 @@ struct ceph_osd_request *ceph_osdc_new_request(struct ceph_osd_client *osdc,
} else
ops[1].op = 0;

- req = ceph_osdc_alloc_request(osdc, snapc, ops,
- use_mempool,
- GFP_NOFS, NULL, NULL);
+ req = ceph_osdc_alloc_request(osdc, snapc, ops, use_mempool, GFP_NOFS);
if (!req)
return ERR_PTR(-ENOMEM);
req->r_flags = flags;

1.7.9.5

These all look good.