Viacheslav Dubeyko [off-list ref] wrote:

quoted

+	struct ceph_databuf *reply;
+	void *p, *q, *end;

If I understood correctly the logic, q represents a pointer on current
position.  So, maybe, it makes sense to rename p into something like
"begin"? In this case, we will have begin pointer, end pointer and p could
be used as the name of pointer on current position.

"hdr" might be a better choice.

quoted

+	iov_iter_advance(&reply->iter, q - p);
 
-	if (offset_in_page(p) + object_map_bytes > reply_len) {
+	if (object_map_bytes > ceph_databuf_len(reply)) {

Does it mean that we had bug before here? Because it was offset_in_page(p) +
object_map_bytes before.

No.  The iov_iter_advance() call advances the iterator over the header which
renders the subtraction unnecessary.

quoted

 	rbd_dev->object_map_size = object_map_size;

Why do we have object_map_size and object_map_bytes at the same time? It could
be confusing for my taste. Maybe, we need to rename the object_map_size to
object_map_num_objects?

Those names preexist.

quoted

+	reply = ceph_databuf_reply_alloc(1, inbound_size, GFP_KERNEL);

Interesting... We allocated memory page before. Now we allocate the memory
of inbound size. Potentially, it could be any size of starting from zero
bytes and including several memory pages. Could we have an issue here?

Shouldn't do.  ceph_databuf_reply_alloc() will expand databuf's bvec[] as
necessary to accommodate sufficient pages for the requested amount of
bufferage.

David