Re: [PATCH liburing v1 2/9] setup: Handle `get_page_size()` failure (for aarch64 nolibc support)

[Jens Axboe <axboe@kernel.dk>]

On 6/28/22 7:00 PM, Ammar Faizi wrote:
> On 6/29/22 7:50 AM, Alviro Iskandar Setiawan wrote:
>> On Wed, Jun 29, 2022 at 7:28 AM Ammar Faizi wrote:
>>>          page_size = get_page_size();
>>> +       if (page_size < 0)
>>> +               return page_size;
>>> +
>>>          return rings_size(p, entries, cq_entries, page_size);
>>>   }
>>
>> the current error handling fallback to 4K if fail on sysconf(_SC_PAGESIZE):
>> https://github.com/axboe/liburing/blob/68103b731c34a9f83c181cb33eb424f46f3dcb94/src/arch/generic/lib.h#L10-L19
>> with this patch, get_page_size() is only possible to return negative
>> value on aarch64.
> 
> Ah right, this one needs a revision. Either we fallback to 4K, or
> return error if we fail.
> 
>> i don't understand why the current master branch code fallback to 4K when fail?
> 
> Neither do I. Maybe because 4K is widely used page size?
> 
> Jens, can you shed some light on this?
> 
>   The current upstream does this:
> 
>      - For x86/x86-64, it's hard-coded to 4K. So it can't fail.
>      - For other archs, if sysconf(_SC_PAGESIZE) fails, we fallback to 4K.
> 
> Now we are going to add aarch64, it uses a group of syscalls to get the page
> size. So it may fail. What should we do when we fail?
> 
> Fallback to 4K? Or return error code from syscall?

4k is the most common page size, by far, so makes sense to have that as
a fallback rather than just error out. Perhaps the application will then
fail differently, but there's also a chance that it'll just work.

So I think just falling back to 4k if we fail for whatever reason is the
sanest recourse.

-- 
Jens Axboe