Skip to content

lib: posix: remove all default POSIX feature enables #88541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

JordanYates
Copy link
Collaborator

When enabling POSIX_API, don't enable any individual features by default. Users can enable the appropriate posix profile through the POSIX_AEP_CHOICE choice, or enable required features individually.

@github-actions github-actions bot added area: Wi-Fi Wi-Fi Release Notes To be mentioned in the release notes labels Apr 12, 2025
@github-actions github-actions bot added the area: POSIX POSIX API Library label Apr 12, 2025
@JordanYates JordanYates marked this pull request as draft April 12, 2025 07:35
@JordanYates JordanYates force-pushed the 250412_posix_cleanup branch 2 times, most recently from 9ce8376 to da74c74 Compare April 12, 2025 12:36
@cfriedt
Copy link
Member

cfriedt commented Apr 12, 2025

This will be good for weeding-out the last remaining users of CONFIG_POSIX_API. I'd like to deprecate that option prior to the next release.

`x509_crt.c` relies on an implementation of `inet_pton`, unless
`MBEDTLS_TEST_SW_INET_PTON` is defined. Since `inet_pton` is only
defined when `POSIX_NETWORKING` is, define the software fallback
if it is not.

Signed-off-by: Jordan Yates <[email protected]>
`http_server_core.c` uses `EVENTFD` functions.

Signed-off-by: Jordan Yates <[email protected]>
`pthread_setspecific` requires a stack in order to allocate the
`struct pthread_key_data` data structure. On 64 bit systems this data
structure is 32 bytes, resulting in 160 bytes usage for the default 5
supported threads.

Signed-off-by: Jordan Yates <[email protected]>
Make `POSIX_AEP_CHOICE` dependent on `POSIX_API`, which enables
removing all the duplicate native library restrictions.

Signed-off-by: Jordan Yates <[email protected]>
Only validate `POSIX_RTSIG_MAX` if `POSIX_REALTIME_SIGNALS` is enabled.

Signed-off-by: Jordan Yates <[email protected]>
Explicitly enable all required POSIX features, instead of relying on
`POSIX_API` to enable defaults.

Signed-off-by: Jordan Yates <[email protected]>
@JordanYates JordanYates force-pushed the 250412_posix_cleanup branch 7 times, most recently from 51c9cea to 4ebcb89 Compare April 14, 2025 00:33
@JordanYates JordanYates marked this pull request as ready for review April 14, 2025 00:34
@nashif
Copy link
Member

nashif commented Apr 14, 2025

it would be nice to not have any of the POSIX kconfig options show up in the .config when POSIX is not enabled, i.e. when I build hello world ( west build -p -b qemu_x86 samples/hello_world/ ) right now and with this PR, I get:

$ grep POSIX build/zephyr/.config

# POSIX API Support
# POSIX Options
# CONFIG_POSIX_API is not set
# CONFIG_POSIX_ASYNCHRONOUS_IO is not set
# CONFIG_POSIX_BARRIERS is not set
# CONFIG_POSIX_C_LANG_SUPPORT_R is not set
# CONFIG_POSIX_C_LIB_EXT is not set
# POSIX device I/O
# CONFIG_POSIX_DEVICE_IO is not set
CONFIG_POSIX_OPEN_MAX=4
# end of POSIX device I/O
# CONFIG_POSIX_FD_MGMT is not set
# CONFIG_POSIX_FILE_SYSTEM_R is not set
# CONFIG_POSIX_FILE_SYSTEM is not set
# POSIX memory
CONFIG_POSIX_PAGE_SIZE=0x1000
# CONFIG_POSIX_SHARED_MEMORY_OBJECTS is not set
# CONFIG_POSIX_MAPPED_FILES is not set
# CONFIG_POSIX_MEMORY_PROTECTION is not set
# end of POSIX memory
# CONFIG_POSIX_MESSAGE_PASSING is not set
# CONFIG_POSIX_SINGLE_PROCESS is not set
# CONFIG_POSIX_MULTI_PROCESS is not set
# CONFIG_POSIX_THREADS is not set
# CONFIG_POSIX_READER_WRITER_LOCKS is not set
# POSIX scheduler options
# CONFIG_POSIX_PRIORITY_SCHEDULING is not set
# end of POSIX scheduler options
# CONFIG_POSIX_SEMAPHORES is not set
# POSIX signals
# CONFIG_POSIX_REALTIME_SIGNALS is not set
# CONFIG_POSIX_SIGNALS is not set
# end of POSIX signals
# CONFIG_POSIX_SPIN_LOCKS is not set
# POSIX synchronized I/O
# CONFIG_POSIX_FSYNC is not set
# CONFIG_POSIX_SYNCHRONIZED_IO is not set
# end of POSIX synchronized I/O
# CONFIG_POSIX_TIMERS is not set
# Miscellaneous POSIX-related options
# end of Miscellaneous POSIX-related options
CONFIG_TC_PROVIDES_POSIX_C_LANG_SUPPORT_R=y
# end of POSIX Options
# POSIX Shell Utilities
# end of POSIX Shell Utilities
# end of POSIX API Support

Why any of this needs to ALWAYS be in the .config if POSIX is not enabled? We do not do this for Bluetooth or for Networking, why do we need to have all those options in the resulting .config if posix is not enabled?

To illustrate this, when you build west build -p -b qemu_x86 samples/bluetooth/central_hr/, you get:

$ grep CONFIG_BT  build/zephyr/.config  | wc -l
259

which is fine, because you we have enabled BT.

when you build hello world:

$ grep CONFIG_BT  build/zephyr/.config
# CONFIG_BT is not set

Thats how it should be

We should do something about this.

@JordanYates
Copy link
Collaborator Author

it would be nice to not have any of the POSIX kconfig options show up in the .config when POSIX is not enabled, i.e. when I build hello world ( west build -p -b qemu_x86 samples/hello_world/ ) right now and with this PR, I get:

I agree with this statement, but as you note this is the current state both on main and in this PR. This PR is trying to limit the number of features that are enabled for users that need some individual piece of the POSIX API, not trying to fix the issue you describe.

@JordanYates JordanYates force-pushed the 250412_posix_cleanup branch 2 times, most recently from 7cad044 to d6b73ee Compare April 14, 2025 05:06
@JordanYates
Copy link
Collaborator Author

@cfriedt The remaining twister test that is failing is already flaky on main:
west build -b qemu_x86_64/atom tests/lib/c_lib/thrd -t run will occasionally hang forever.

@jukkar
Copy link
Member

jukkar commented Apr 14, 2025

Why do we need to manually enable CONFIG_POSIX_NETWORKING everywhere? Couldn't we select it automatically if CONFIG_NETWORKING and CONFIG_POSIX_API are set?

@JordanYates
Copy link
Collaborator Author

JordanYates commented Apr 14, 2025

Why do we need to manually enable CONFIG_POSIX_NETWORKING everywhere? Couldn't we select it automatically if CONFIG_NETWORKING and CONFIG_POSIX_API are set?

It is not a given that everyone that uses NETWORKING and POSIX_API also wants POSIX_NETWORKING. Maybe they only want POSIX_THREADS, or POSIX_COUNTERS, or w/e. There is also the consideration that @cfriedt wants to deprecate POSIX_API entirely, so there is the question of what symbol are you enabling POSIX_NETWORKING based on?

@jukkar
Copy link
Member

jukkar commented Apr 14, 2025

There is also the consideration that @cfriedt wants to deprecate POSIX_API entirely

Yes, I have heard about this. I have not yet understood why as the POSIX_API could act as a master switch for enabling Posix support. So similar as what CONFIG_NETWORKING does for net.

It is not a given that everyone that uses NETWORKING and POSIX_API also wants POSIX_NETWORKING.

Perhaps we could do imply then and allow user to disable POSIX_NETWORKING if needed. I am just trying to brainstorm here to avoid this large number of changes in this PR.

@nashif
Copy link
Member

nashif commented Apr 14, 2025

it would be nice to not have any of the POSIX kconfig options show up in the .config when POSIX is not enabled, i.e. when I build hello world ( west build -p -b qemu_x86 samples/hello_world/ ) right now and with this PR, I get:

I agree with this statement, but as you note this is the current state both on main and in this PR. This PR is trying to limit the number of features that are enabled for users that need some individual piece of the POSIX API, not trying to fix the issue you describe.

right, not suggesting this should be done as part of this PR :)
There is a bug about this already: #75843

btw, there is another issue with how kconfigs of posix are implemented, for example I can now do:

west build -p -b qemu_x86 samples/hello_world/ -- -DCONFIG_POSIX_TIMERS=y and nothing prevents me from enabling POSIX_TIMERS directly, but this Kconfig has no dependencies whatsoever, which in this case leads to compile errors. We should not be able to configure such options if the underlying subsystem is not enabled.

Explicitly enable the features required for the tests to pass, rather
than relying on `POSIX_API` to enable defaults.

Signed-off-by: Jordan Yates <[email protected]>
Explicitly enable all required POSIX features, instead of relying on
`POSIX_API` to enable defaults.

Signed-off-by: Jordan Yates <[email protected]>
When enabling `POSIX_API`, don't enable any individual features by
default. Users can enable the appropriate posix profile through the
`POSIX_AEP_CHOICE` choice, or enable required features individually.

Signed-off-by: Jordan Yates <[email protected]>
@JordanYates JordanYates force-pushed the 250412_posix_cleanup branch from d6b73ee to fc238c5 Compare April 14, 2025 10:57
@cfriedt
Copy link
Member

cfriedt commented Apr 14, 2025

it would be nice to not have any of the POSIX kconfig options show up in the .config when POSIX is not enabled

@nashif - I have a PR in draft this address that right now. Should be merged before 4.2.0, I think. #88547

Also, @jukkar - please see the above PR. Should offer clarity.

@cfriedt
Copy link
Member

cfriedt commented Apr 14, 2025

There is also the consideration that @cfriedt wants to deprecate POSIX_API entirely, so there is the question of what symbol are you enabling POSIX_NETWORKING based on?

In #88547, POSIX Option Groups that do not require involvement from the os (e.g. string manipulation functions) are separated from POSIX Option Groups that do require OS involvement.

This allows us to very clearly gate most POSIX Option Groups with a Kconfig, removing most of them entirely from e.g. hello world.

I chose CONFIG_POSIX_SYSTEM_INTERFACES because I didn't like the idea of reusing CONFIG_POSIX_API with it's ad-hoc defaults and the baggage that comes with that, but naming for this new option is flexible. The main idea was to deprecate CONFIG_POSIX_API gracefully according to the process. We could reuse that option if the arch wg thinks that's a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: POSIX POSIX API Library area: Wi-Fi Wi-Fi Release Notes To be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants