[libbsd 00/22] Remove FreeBSD file descriptors and avoid VFS

Fri Oct 21 06:53:43 UTC 2022

On 21/10/2022 02:30, Chris Johns wrote:
> On 20/10/2022 7:01 pm, Sebastian Huber wrote:
>> On 29/08/2022 10:27, Sebastian Huber wrote:
>>> Hello Chris,
>>>
>>> On 25/07/2022 08:12, Sebastian Huber wrote:
>>>> On 11/07/2022 15:04, Sebastian Huber wrote:
>>>>> On 24/06/2022 08:33, Sebastian Huber wrote:
>>>>>> This patch set removes the FreeBSD file descriptors.  The VFS is no longer
>>>>>> used
>>>>>> if only the USB, SD/MMC, network, PCI, and NVMe support is used by the
>>>>>> application.  This change significantly reduce the memory usage of LibBSD for
>>>>>> these applications.  Using the media01 test case for the arm/lpc32xx BSP as a
>>>>>> benchmark, the heap usage dropped from 14.3MiB to 10.2MiB.  The "_BSD
>>>>>> bufdaemon", "_BSD vnlru", "_BSD syncer", and "_BSD bufspacedaemon-" tasks are
>>>>>> no longer present in media01.  The code size is reduced by about 8KiB.  The
>>>>>> data size is reduced by about 30KiB.  The throughput with a simple FTP test
>>>>>> increased by about 1%.
>>>>>>
>>>>>> The "Remove FreeBSD file descriptors" change removes more lines than there are
>>>>>> added.
>>>>>>
>>>>>> This change makes it easier to port the NFS support to the master branch since
>>>>>> now the changes are more localized.
>>>>>
>>>>> I have a target with only 8MiB of RAM (for code and data). So, this patch
>>>>> set is not just a micro optimization.
>>>>
>>>> This pending patch set is currently a blocker for me. I would like to work on
>>>> the NTP daemon integration and an update of the libbsd master branch to a
>>>> more recent FreeBSD version. This patch set just restores what worked well
>>>> for several years.
>>>
>>> did you have time to review this patch set in the meantime? The NTP client no
>>> longer crashes with this patch set.
>>
>> This patch set is now pending for about four months. From my point of view it is
>> a straight forward fix of some severe issues which prevent the use of libbsd on
>> lower end targets.
> 
> Is there a repo somewhere with the patches applied? I am being lazy asking as I
> would like to avoid downloading the 22 patches and applying them by hand.

Thanks for having a look at this work. You find a branch for testing here:

https://github.com/sebhub/rtems-libbsd/tree/filedesc

You need this RTEMS patch:

https://lists.rtems.org/pipermail/devel/2022-June/072055.html

> 
> It seems the media test cannot grow by more than 9K of code and the heap by 5M!
> Is my understanding correct? Anything else we need to understand about the low
> end limits?

If you have only 16M or 32M of RAM for code and data than adding 5M for 
nothing is a big deal. The libbsd started as a libusb. Not every 
application uses the network stuff.

> 
> I have not seen any requirements on the lower end for libbsd before now. How do
> we manage the competing requirement of low end support and adding functionality
> to libbsd in the future if they clash?
> 
> Adding VFS support as FreeBSD implements it has pushed you over the edge but
> that edge is not defined so it makes it difficult for anyone other than you to
> work on this code base in a major way. I was not aware this was an issue and for
> me the extra 9K of code is not a problem and a 5M change in heap is acceptable.

The problem is not the VFS support. The problem is that you added the 
FreeBSD file descriptors and concentrated all the system calls in one 
translation unit.

> 
>> It is not really good if our internal libbsd branches diverge
>> too much from the RTEMS mainline.
> 
> Yes I agree. It is not in anyone's interest and that only benefits you.
> 
> I would like to reiterate we have an obligation to maintain the code base as
> close to FreeBSD as possible. Some of these changes push that boundary. We need
> to find a suitable path forward.

Being as close as possible to FreeBSD should not result in being 
FreeBSD. RTEMS has its own infrastructure and one part is the file 
descriptor handling. There are always trade-offs to consider when 
porting new components from FreeBSD to RTEMS.

> 
> Reviewing the changes:
> 
> 1. The break up of the syscalls file. The related comments are:
> 
>   "Collecting all system calls in a single translation unit is not good
>    due to the library initialization through linker sets."
> 
> Could you please explain what the linker set issue is?

The libbsd uses a constructor based initialization. The constructors are 
pulled in through the configuration and indirectly by using API level 
functions. If you concentrate the API level function in one file, then 
everything related to all of the API level functions is pulled in and 
initialized. Therefore, the API level functions need to be placed in 
separate translation units so that you get only what you need in your 
executable. For example, if you just want to use USB and sysctl(), then 
you should not end up with stuff related to socket().

> 
> Why the need to change FreeBSD calls to static? Is it solely a performance
> optimisation?

It is also easier to maintain if you have the RTEMS glue code attached 
to the FreeBSD system call implementation (sys_* and kern_*). For 
example, in uipc_syscalls.c the glue code was placed in white space 
between functions. Such code areas are absolutely no issue during 
updates. I did some updates of the FreeBSD base line, so this is not 
just theoretic reasoning.

> 
> We are required to use function and data sections so I assume the syscall file
> will be broken up by the linker where possible?

Yes, but the problem is that before the linker garbage collection takes 
place all symbols are resolved. This pulls in the constructors. The 
constructors cannot be garbage collected, so all their dependencies 
remain in the executable.

> 
> I cannot tell from the patches if the conditional syscall trace support is still
> present?

It is no longer included. This could be added later, but why do we need 
another tracing solution? Can't the trace linker do this tracing?

> 
> I do not agree with your statement in the patches separation helps porting. My
> experience was not that. When adding VFS and the NFS client I found the syscall
> code all over the place and not easy to find, hard to review and it added
> complexity. It adds unnecessary changes to the FreeBSD code against a base
> requirement of the porting effort.

It is all other the place because FreeBSD is modular. What do you mean 
by added complexity? Why is it hard to review if you have the socket() 
implementation directly below sys_socket(), etc.?

> 
> 2. FreeBSD file descriptor removal
> 
> Does this mean we can never have them in libbsd? Does that limit what can be
> brought across from FreeBSD in the future?

I see absolutely no need for using the FreeBSD file descriptors. If we 
are limited by the RTEMS file descriptors, then we can change this. For 
example, the poll and kqfilter handlers were added to support libbsd 
features.

> 
> 3. select, poll, and kqueue
> 
> Do these calls work with an NFS client fd?

Is there a test for this in the test suite? The VFS support uses:

int
rtems_bsd_sysgen_kqfilter(rtems_libio_t *iop, struct knote *kn)
{
	struct file *fp;

	fp = iop->data1;
	return (fo_kqfilter(fp, kn));
}

> 
> 4. Types of RTEMS fd's in libbsd
> 
> The change I made brought the types into a single location so it is easier to
> review and understand. There is:
> 
>   - rtems_bsd_sysgen_dirops
>   - rtems_bsd_sysgen_fileops
>   - rtems_bsd_sysgen_nodeops
>   - rtems_bsd_sysgen_imfsnodeops
> 
> VFS is now left to handle the specific vnode ops as FreeBSD is designed to do.
> 
> [
> https://git.rtems.org/rtems-libbsd/tree/rtemsbsd/rtems/rtems-bsd-syscall-api.c?h=6-freebsd-12
> ]

The patch set doesn't change this this. The VFS support is still in 
rtems-bsd-syscall-api.c and this code is actually simpler now. The VFS 
support and the FreeBSD file descriptors are two separate things.

The FreeBSD file descriptors added quite a bit of complexity due to the 
extra layer. This was absolutely not easy to understand. I bumped into 
this very issue while having to debug a problem with the NTP support.

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber at embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/