Re: [RFC PATCH 21/35] libceph: Make notify code use ceph_databuf_enc_start/stop

David Howells
2025-03-18T22:36:41Z

Viacheslav Dubeyko <Slava.Dubeyko@ibm.com> wrote:

> > @@ -4852,20 +4854,19 @@ int ceph_osdc_notify(struct ceph_osd_client *osdc,
> > if (!lreq)
> > return -ENOMEM;
> >
> > - lreq->request_pl = ceph_databuf_req_alloc(1, PAGE_SIZE, GFP_NOIO);
> > + lreq->request_pl = ceph_databuf_req_alloc(0, 3 * 4 + payload_len,
>
> The same question... :)
>
> Thanks,
> Slava.
>
> > + GFP_NOIO);
> > if (!lreq->request_pl) {
> > ret = -ENOMEM;
> > goto out_put_lreq;
> > }
> >
> > - ret = ceph_databuf_encode_32(lreq->request_pl, 1); /* prot_ver */
> > - ret |= ceph_databuf_encode_32(lreq->request_pl, timeout);
> > - ret |= ceph_databuf_encode_32(lreq->request_pl, payload_len);
> > - ret |= ceph_databuf_append(lreq->request_pl, payload, payload_len);
> > - if (ret) {
> > - ret = -ENOMEM;
> > - goto out_put_lreq;
> > - }
> > + p = ceph_databuf_enc_start(lreq->request_pl);
> > + ceph_encode_32(&p, 1); /* prot_ver */
> > + ceph_encode_32(&p, timeout);
> > + ceph_encode_32(&p, payload_len);
> > + ceph_encode_copy(&p, payload, payload_len);
> > + ceph_databuf_enc_stop(lreq->request_pl, p);

I think the answer is obvious from that. You have 3 protocol LE32 words plus
the payload. Previously, ceph just allocated a whole page, whether or not we
needed anywhere near that much, and then would dynamically add pages as it
went along if one was insufficient. By allocating up front, we get to make
use of the bulk allocator.

However, if we don't need all that much space, it affords us the opportunity
to make use of a page fragment allocator.

David