On Sun, 16 Jun 2024 19:04:38 +0100, Pavel Begunkov wrote:
> On 5/14/24 08:54, Chenliang Li wrote:
>> Introduce helper functions to check whether a buffer can
>> be coalesced or not, and gather folio data for later use.
>>
>> The coalescing optimizes time and space consumption caused
>> by mapping and storing multi-hugepage fixed buffers.
>>
>> A coalescable multi-hugepage buffer should fully cover its folios
>> (except potentially the first and last one), and these folios should
>> have the same size. These requirements are for easier later process,
>> also we need same size'd chunks in io_import_fixed for fast iov_iter
>> adjust.
>>
>> Signed-off-by: Chenliang Li <cliang01.li@xxxxxxxxxxx>
>> ---
>> io_uring/rsrc.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++
>> io_uring/rsrc.h | 10 +++++++
>> 2 files changed, 88 insertions(+)
>>
>> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
>> index 65417c9553b1..d08224c0c5b0 100644
>> --- a/io_uring/rsrc.c
>> +++ b/io_uring/rsrc.c
>> @@ -871,6 +871,84 @@ static int io_buffer_account_pin(struct io_ring_ctx *ctx, struct page **pages,
>> return ret;
>> }
>>
>> +static bool __io_sqe_buffer_try_coalesce(struct page **pages, int nr_pages,
>> + struct io_imu_folio_data *data)
> io_can_coalesce_buffer(), you're not actually trying to
> do it here.
Will change it.
>> +static bool io_sqe_buffer_try_coalesce(struct page **pages, int nr_pages,
>> + struct io_imu_folio_data *data)
>> +{
>> + int i, j;
>> +
>> + if (nr_pages <= 1 ||
>> + !__io_sqe_buffer_try_coalesce(pages, nr_pages, data))
>> + return false;
>> +
>> + /*
>> + * The pages are bound to the folio, it doesn't
>> + * actually unpin them but drops all but one reference,
>> + * which is usually put down by io_buffer_unmap().
>> + * Note, needs a better helper.
>> + */
>> + if (data->nr_pages_head > 1)
>> + unpin_user_pages(&pages[1], data->nr_pages_head - 1);
> Should be pages[0]. page[1] can be in another folio, and even
> though data->nr_pages_head > 1 protects against touching it,
> it's still flimsy.
But here it is unpinning the tail pages inside those coalesceable folios,
I think we only unpin pages[0] when failure, am I right? And in
__io_sqe_buffer_try_coalesce we have ensured that pages[1:nr_head_pages] are
in same folio and contiguous.
>> +
>> + j = data->nr_pages_head;
>> + nr_pages -= data->nr_pages_head;
>> + for (i = 1; i < data->nr_folios; i++) {
>> + unsigned int nr_unpin;
>> +
>> + nr_unpin = min_t(unsigned int, nr_pages - 1,
>> + data->nr_pages_mid - 1);
>> + if (nr_unpin == 0)
>> + break;
>> + unpin_user_pages(&pages[j+1], nr_unpin);
> same
>> + j += data->nr_pages_mid;
> And instead of duplicating this voodoo iteration later,
> please just assemble a new compacted ->nr_folios sized
> page array.
Indeed, a new page array would make things a lot easier.
If alloc overhead is not a concern here, then yeah I'll change it.
Thanks,
Chenliang Li
