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.hu...@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/
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Reply via email to