Re: [PATCHSET v1 RFC liburing 0/6] Add no libc support for x86-64 arch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/6/21 4:20 PM, Ammar Faizi wrote:
> On Thu, Oct 7, 2021 at 1:48 AM Jens Axboe <axboe@xxxxxxxxx> wrote:
>> On 10/6/21 8:49 AM, Ammar Faizi wrote:
>>> Hi everyone,
>>>
>>> This is the v1 of RFC to support build liburing without libc.
>>>
>>> In this RFC, I introduce no libc support for x86-64 arch. Hopefully,
>>> one day we can get support for other architectures as well.
>>>
>>> Motivation:
>>> Currently liburing depends on libc. We want to make liburing can be
>>> built without libc.
>>>
>>> This idea firstly posted as an issue on the liburing GitHub
>>> repository here: https://github.com/axboe/liburing/issues/443
>>>
>>> The subject of the issue is: "An option to use liburing without libc?".
>>
>> This series seems to be somewhat upside down. You should fix up tests
>> first, then add support for x86-64 nolibc, then enable it. You seem to
>> be doing the opposite?
>>
>> -- 
>> Jens Axboe
> 
> Yes, that's what I am doing.
> 
> I agree with add support for x86-64 nolibc, then enable it. However,
> the tests fixes happened very naturally.
> 
> I would not be able to caught those broken tests if I didn't add the
> nolibc support.

Right, but that's exactly why they should get fixed up front! If not,
you've got this weird point in the git tree that doesn't make any sense.
Each patch, when applied, should leave you with a fully functional repo.

> There are two main problems with the tests, all of them are caught
> after adding no libc support.
> 
>   1) The test uses `errno` to check error from liburing functions,
>      this is only problematic with no libc build. I wouldn't be able
>      to caught this without adding no libc support. I caught several
>      tests failed after added no libc support, then investigated the
>      failure causes and found the culprit (it's errno from the libc).
> 
>   2) The test uses `free()` to free the return value of
>      `io_uring_get_probe{,_ring}` functions
>      from liburing. This causes invalid free only for nolibc build.
>      So it does really make sense to add no libc support first, then
>      fix up the tests. Because there is no way I can know this broken
>      situation earlier.

2 is just a plain bug, and was introduced when that probe freeing
function was added. So that's definitely just a separate upfront patch
to fix that.

1 can be fixed too upfront, as it should not cause any changes in
behavior.

> Since now I know everything about the situation, I can do so. So I
> will send the RFC v2 and rebase everything based on your order.

Thanks!

-- 
Jens Axboe




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux