Re: [RFC PATCH 18/35] libceph, rbd: Convert some page arrays to ceph_databuf

[Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>]

On Thu, 2025-03-13 at 23:33 +0000, David Howells wrote:
> Convert some miscellaneous page arrays to ceph_databuf containers.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Viacheslav Dubeyko <slava@dubeyko.com>
> cc: Alex Markuze <amarkuze@redhat.com>
> cc: Ilya Dryomov <idryomov@gmail.com>
> cc: ceph-devel@vger.kernel.org
> cc: linux-fsdevel@vger.kernel.org
> ---
>  drivers/block/rbd.c             | 12 ++++-----
>  include/linux/ceph/osd_client.h |  3 +++
>  net/ceph/osd_client.c           | 43 +++++++++++++++++++++------------
>  3 files changed, 36 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 078bb1e3e1da..eea12c7ab2a0 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2108,7 +2108,7 @@ static int rbd_obj_calc_img_extents(struct rbd_obj_request *obj_req,
>  
>  static int rbd_osd_setup_stat(struct ceph_osd_request *osd_req, int which)
>  {
> -	struct page **pages;
> +	struct ceph_databuf *dbuf;
>  
>  	/*
>  	 * The response data for a STAT call consists of:
> @@ -2118,14 +2118,12 @@ static int rbd_osd_setup_stat(struct ceph_osd_request *osd_req, int which)
>  	 *         le32 tv_nsec;
>  	 *     } mtime;
>  	 */
> -	pages = ceph_alloc_page_vector(1, GFP_NOIO);
> -	if (IS_ERR(pages))
> -		return PTR_ERR(pages);
> +	dbuf = ceph_databuf_reply_alloc(1, 8 + sizeof(struct ceph_timespec), GFP_NOIO);

What this 8 + sizeof(struct ceph_timespec) means? Why do we use 8 here? :)

Thanks,
Slava.

> +	if (!dbuf)
> +		return -ENOMEM;
>  
>  	osd_req_op_init(osd_req, which, CEPH_OSD_OP_STAT, 0);
> -	osd_req_op_raw_data_in_pages(osd_req, which, pages,
> -				     8 + sizeof(struct ceph_timespec),
> -				     0, false, true);
> +	osd_req_op_raw_data_in_databuf(osd_req, which, dbuf);
>  	return 0;
>  }
>  
> diff --git a/include/linux/ceph/osd_client.h b/include/linux/ceph/osd_client.h
> index d31e59bd128c..6e126e212271 100644
> --- a/include/linux/ceph/osd_client.h
> +++ b/include/linux/ceph/osd_client.h
> @@ -482,6 +482,9 @@ extern void osd_req_op_extent_osd_data_pages(struct ceph_osd_request *,
>  					struct page **pages, u64 length,
>  					u32 offset, bool pages_from_pool,
>  					bool own_pages);
> +void osd_req_op_raw_data_in_databuf(struct ceph_osd_request *osd_req,
> +				    unsigned int which,
> +				    struct ceph_databuf *databuf);
>  extern void osd_req_op_extent_osd_data_pagelist(struct ceph_osd_request *,
>  					unsigned int which,
>  					struct ceph_pagelist *pagelist);
> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
> index b4adb299f9cd..64a06267e7b3 100644
> --- a/net/ceph/osd_client.c
> +++ b/net/ceph/osd_client.c
> @@ -182,6 +182,17 @@ osd_req_op_extent_osd_data(struct ceph_osd_request *osd_req,
>  }
>  EXPORT_SYMBOL(osd_req_op_extent_osd_data);
>  
> +void osd_req_op_raw_data_in_databuf(struct ceph_osd_request *osd_req,
> +				    unsigned int which,
> +				    struct ceph_databuf *dbuf)
> +{
> +	struct ceph_osd_data *osd_data;
> +
> +	osd_data = osd_req_op_raw_data_in(osd_req, which);
> +	ceph_osd_databuf_init(osd_data, dbuf);
> +}
> +EXPORT_SYMBOL(osd_req_op_raw_data_in_databuf);
> +
>  void osd_req_op_raw_data_in_pages(struct ceph_osd_request *osd_req,
>  			unsigned int which, struct page **pages,
>  			u64 length, u32 offset,
> @@ -5000,7 +5011,7 @@ int ceph_osdc_list_watchers(struct ceph_osd_client *osdc,
>  			    u32 *num_watchers)
>  {
>  	struct ceph_osd_request *req;
> -	struct page **pages;
> +	struct ceph_databuf *dbuf;
>  	int ret;
>  
>  	req = ceph_osdc_alloc_request(osdc, NULL, 1, false, GFP_NOIO);
> @@ -5011,16 +5022,16 @@ int ceph_osdc_list_watchers(struct ceph_osd_client *osdc,
>  	ceph_oloc_copy(&req->r_base_oloc, oloc);
>  	req->r_flags = CEPH_OSD_FLAG_READ;
>  
> -	pages = ceph_alloc_page_vector(1, GFP_NOIO);
> -	if (IS_ERR(pages)) {
> -		ret = PTR_ERR(pages);
> +	dbuf = ceph_databuf_reply_alloc(1, PAGE_SIZE, GFP_NOIO);
> +	if (!dbuf) {
> +		ret = -ENOMEM;
>  		goto out_put_req;
>  	}
>  
>  	osd_req_op_init(req, 0, CEPH_OSD_OP_LIST_WATCHERS, 0);
> -	ceph_osd_data_pages_init(osd_req_op_data(req, 0, list_watchers,
> -						 response_data),
> -				 pages, PAGE_SIZE, 0, false, true);
> +	ceph_osd_databuf_init(osd_req_op_data(req, 0, list_watchers,
> +					      response_data),
> +			      dbuf);
>  
>  	ret = ceph_osdc_alloc_messages(req, GFP_NOIO);
>  	if (ret)
> @@ -5029,10 +5040,11 @@ int ceph_osdc_list_watchers(struct ceph_osd_client *osdc,
>  	ceph_osdc_start_request(osdc, req);
>  	ret = ceph_osdc_wait_request(osdc, req);
>  	if (ret >= 0) {
> -		void *p = page_address(pages[0]);
> +		void *p = kmap_ceph_databuf_page(dbuf, 0);
>  		void *const end = p + req->r_ops[0].outdata_len;
>  
>  		ret = decode_watchers(&p, end, watchers, num_watchers);
> +		kunmap(p);
>  	}
>  
>  out_put_req:
> @@ -5246,12 +5258,12 @@ int osd_req_op_copy_from_init(struct ceph_osd_request *req,
>  			      u8 copy_from_flags)
>  {
>  	struct ceph_osd_req_op *op;
> -	struct page **pages;
> +	struct ceph_databuf *dbuf;
>  	void *p, *end;
>  
> -	pages = ceph_alloc_page_vector(1, GFP_KERNEL);
> -	if (IS_ERR(pages))
> -		return PTR_ERR(pages);
> +	dbuf = ceph_databuf_req_alloc(1, PAGE_SIZE, GFP_KERNEL);
> +	if (!dbuf)
> +		return -ENOMEM;
>  
>  	op = osd_req_op_init(req, 0, CEPH_OSD_OP_COPY_FROM2,
>  			     dst_fadvise_flags);
> @@ -5260,16 +5272,17 @@ int osd_req_op_copy_from_init(struct ceph_osd_request *req,
>  	op->copy_from.flags = copy_from_flags;
>  	op->copy_from.src_fadvise_flags = src_fadvise_flags;
>  
> -	p = page_address(pages[0]);
> +	p = kmap_ceph_databuf_page(dbuf, 0);
>  	end = p + PAGE_SIZE;
>  	ceph_encode_string(&p, src_oid->name, src_oid->name_len);
>  	encode_oloc(&p, src_oloc);
>  	ceph_encode_32(&p, truncate_seq);
>  	ceph_encode_64(&p, truncate_size);
>  	op->indata_len = PAGE_SIZE - (end - p);
> +	ceph_databuf_added_data(dbuf, op->indata_len);
> +	kunmap_local(p);
>  
> -	ceph_osd_data_pages_init(&op->copy_from.osd_data, pages,
> -				 op->indata_len, 0, false, true);
> +	ceph_osd_databuf_init(&op->copy_from.osd_data, dbuf);
>  	return 0;
>  }
>  EXPORT_SYMBOL(osd_req_op_copy_from_init);
> 
>