On 5/10/24 11:52 PM, Chenliang Li wrote:
> This patch depends on patch 1 and 2.
What does "patch 1 and 2" mean here, once it's in the git log? It
doesn't really mean anything. It's quite natural for patches in a series
to have dependencies on each other, eg patch 3 requirest 1 and 2.
Highlighting it doesn't really add anything, so just get rid of that.
> Introduces two functions to separate the coalesced imu alloc and
> accounting path from the original one. This helps to keep the original
> code path clean.
>
> Signed-off-by: Chenliang Li <cliang01.li@xxxxxxxxxxx>
> ---
> io_uring/rsrc.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 86 insertions(+)
>
> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> index 578d382ca9bc..7f95eba72f1c 100644
> --- a/io_uring/rsrc.c
> +++ b/io_uring/rsrc.c
> @@ -871,6 +871,42 @@ static int io_buffer_account_pin(struct io_ring_ctx *ctx, struct page **pages,
> return ret;
> }
>
> +static int io_coalesced_buffer_account_pin(struct io_ring_ctx *ctx,
> + struct page **pages,
> + struct io_mapped_ubuf *imu,
> + struct page **last_hpage,
> + struct io_imu_folio_data *data)
> +{
> + int i, j, ret;
> +
> + imu->acct_pages = 0;
> + j = 0;
> + for (i = 0; i < data->nr_folios; i++) {
> + struct page *hpage = pages[j];
> +
> + if (hpage == *last_hpage)
> + continue;
> + *last_hpage = hpage;
> + /*
> + * Already checked the page array in try coalesce,
> + * so pass in nr_pages=0 here to waive that.
> + */
> + if (headpage_already_acct(ctx, pages, 0, hpage))
> + continue;
> + imu->acct_pages += data->nr_pages_mid;
> + j += (i == 0) ?
> + data->nr_pages_head : data->nr_pages_mid;
Can we just initialize 'j' to data->nr_pages_head and change this to be:
if (i)
j += data->nr_pages_mid;
That would be a lot cleaner.
> + if (!imu->acct_pages)
> + return 0;
> +
> + ret = io_account_mem(ctx, imu->acct_pages);
> + if (ret)
> + imu->acct_pages = 0;
> + return ret;
> +}
ret = io_account_mem(ctx, imu->acct_pages);
if (!ret)
return 0;
imu->acct_pages = 0;
return ret;
> + struct io_mapped_ubuf **pimu,
> + struct page **last_hpage, struct page **pages,
> + struct io_imu_folio_data *data)
> +{
> + struct io_mapped_ubuf *imu = NULL;
> + unsigned long off;
> + size_t size, vec_len;
> + int ret, i, j;
> +
> + ret = -ENOMEM;
> + imu = kvmalloc(struct_size(imu, bvec, data->nr_folios), GFP_KERNEL);
> + if (!imu)
> + return ret;
> +
> + ret = io_coalesced_buffer_account_pin(ctx, pages, imu, last_hpage,
> + data);
> + if (ret) {
> + j = 0;
> + for (i = 0; i < data->nr_folios; i++) {
> + unpin_user_page(pages[j]);
> + j += (i == 0) ?
> + data->nr_pages_head : data->nr_pages_mid;
> + }
> + return ret;
Same comment here.
--
Jens Axboe
