Re: [PATCH v1 2/3] io_uring/rsrc: don't rely on user vaddr alignment

[Pavel Begunkov <asml.silence@gmail.com>]

On 6/24/25 13:42, David Hildenbrand wrote:
> On 24.06.25 12:35, Pavel Begunkov wrote:
>> There is no guaranteed alignment for user pointers, however the
>> calculation of an offset of the first page into a folio after
>> coalescing uses some weird bit mask logic, get rid of it.
>>
>> Cc: stable@vger.kernel.org
>> Reported-by: David Hildenbrand <david@redhat.com>
>> Fixes: a8edbb424b139 ("io_uring/rsrc: enable multi-hugepage buffer coalescing")
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>> ---
>>   io_uring/rsrc.c | 8 +++++++-
>>   io_uring/rsrc.h | 1 +
>>   2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
>> index e83a294c718b..5132f8df600f 100644
>> --- a/io_uring/rsrc.c
>> +++ b/io_uring/rsrc.c
>> @@ -734,6 +734,8 @@ bool io_check_coalesce_buffer(struct page **page_array, int nr_pages,
>>       data->nr_pages_mid = folio_nr_pages(folio);
>>       data->folio_shift = folio_shift(folio);
>> +    data->first_page_offset = page_array[0] - compound_head(page_array[0]);
> 
> Note: pointer arithmetic on "struct page" does not work reliably for very large folios (eg., 1 GiB hugetlb) in all configs (!CONFIG_SPARSEMEM_VMEMMAP)
> 
> I assume this can be
> 
> data->first_page_offset = folio_page_idx(folio, page_array[0]);

Yep, already changed it in v2

...>> diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h
>> index 0d2138f16322..d823554a8817 100644
>> --- a/io_uring/rsrc.h
>> +++ b/io_uring/rsrc.h
>> @@ -49,6 +49,7 @@ struct io_imu_folio_data {
>>       unsigned int    nr_pages_mid;
>>       unsigned int    folio_shift;
>>       unsigned int    nr_folios;
>> +    unsigned long    first_page_offset;
> 
> Heh, is it actually "first_folio_offset" ?

It's an offset into the folio, or the offset of the page
in the folio, and first_folio_offset can also be interpreted
a wrong way. The usual naming problem

> Alternatively, call it "first_folio_page_idx" or sth like that and leave the shift to the user.

Let's do that

-- 
Pavel Begunkov