Skip to content

Conversation

@andrewboie
Copy link
Contributor

@andrewboie andrewboie commented Mar 12, 2020

  • Poorly named structs _k_object, _k_thread_stack_element, _k_object_assignment renamed to use z_ prefix, all are kernel-private
  • The data member of struct z_object is now a union making the semantics much clearer
  • Generation of privilege mode stacks is no longer a separate pass, logic merged into gen_kobject_list.py. this should speed up build times on ARM platforms.
  • elf_helper.py merged back into gen_kobject_list.py

Fixes: #15304

@zephyrbot
Copy link

zephyrbot commented Mar 12, 2020

All checks are passing now.

checkpatch (informational only, not a failure)

-:932: WARNING:NEW_TYPEDEFS: do not add new typedefs
#932: FILE: include/sys/arch_interface.h:44:
+typedef struct z_thread_stack_element k_thread_stack_t;

- total: 0 errors, 1 warnings, 1871 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@andrewboie andrewboie force-pushed the userspace-cleanups branch 3 times, most recently from a7561ab to cda7887 Compare March 12, 2020 20:45
@andrewboie andrewboie force-pushed the userspace-cleanups branch 3 times, most recently from 0814a45 to a46d3f6 Compare March 13, 2020 17:29
@andrewboie
Copy link
Contributor Author

This seems to have broken make clean; make, investigating...

Traceback (most recent call last):
  File "/home/apboie/projects/zephyr2/zephyr/scripts/subfolder_list.py", line 80, in <module>
    main()
  File "/home/apboie/projects/zephyr2/zephyr/scripts/subfolder_list.py", line 44, in main
    os.symlink(directory, symlink)
FileExistsError: [Errno 17] File exists: '/home/apboie/projects/zephyr2/zephyr/include' -> '/home/apboie/projects/zephyr2/zephyr/tests/kernel/mem_protect/userspace/out/zephyr/misc/generated/syscalls_links/_home_apboie_projects_zephyr2_zephyr_include'
Traceback (most recent call last):
  File "/home/apboie/projects/zephyr2/zephyr/scripts/subfolder_list.py", line 80, in <module>
    main()
  File "/home/apboie/projects/zephyr2/zephyr/scripts/subfolder_list.py", line 44, in main
    os.symlink(directory, symlink)
FileExistsError: [Errno 17] File exists: '/home/apboie/projects/zephyr2/zephyr/include' -> '/home/apboie/projects/zephyr2/zephyr/tests/kernel/mem_protect/userspace/out/zephyr/misc/generated/syscalls_links/_home_apboie_projects_zephyr2_zephyr_include'
zephyr/CMakeFiles/kobj_types_h_target.dir/build.make:168: recipe for target 'zephyr/misc/generated/syscalls_subdirs.trigger' failed
make[2]: *** [zephyr/misc/generated/syscalls_subdirs.trigger] Error 1
make[2]: *** Deleting file 'zephyr/misc/generated/syscalls_subdirs.trigger'
CMakeFiles/Makefile2:733: recipe for target 'zephyr/CMakeFiles/kobj_types_h_target.dir/all' failed
make[1]: *** [zephyr/CMakeFiles/kobj_types_h_target.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
[  5%] Generating include/generated/syscall_dispatch.c, include/generated/syscall_list.h
zephyr/CMakeFiles/driver_validation_h_target.dir/build.make:164: recipe for target 'zephyr/misc/generated/syscalls_subdirs.trigger' failed
make[2]: *** [zephyr/misc/generated/syscalls_subdirs.trigger] Error 1
CMakeFiles/Makefile2:765: recipe for target 'zephyr/CMakeFiles/driver_validation_h_target.dir/all' failed
make[1]: *** [zephyr/CMakeFiles/driver_validation_h_target.dir/all] Error 2
[  5%] Built target syscall_list_h_target
Makefile:83: recipe for target 'all' failed
make: *** [all] Error 2

@andrewboie
Copy link
Contributor Author

@SebastianBoe I'm not sure what part of my CMakeLists.txt changes broke this, but I think there is an underlying bug -- even in a vanilla Zephyr tree, when I run "make clean" there's still tons of generated stuff left in build/zephyr/include/generated

@andrewboie andrewboie requested a review from dcpleung March 13, 2020 21:31
@SebastianBoe
Copy link
Contributor

I was not able to reproduce the issue of make clean not working.

But I see that this happens:

[  1%] Preparing syscall dependency handling
[  1%] Preparing syscall dependency handling
[  2%] Preparing syscall dependency handling



[  3%] Generating misc/generated/syscalls_subdirs.trigger
[  4%] Generating misc/generated/syscalls_subdirs.trigger
[  5%] Generating misc/generated/syscalls_subdirs.trigger
[  6%] Generating misc/generated/syscalls.json, misc/generated/subsystems.json
[  7%] Generating misc/generated/syscalls.json, misc/generated/subsystems.json
[  7%] Generating misc/generated/syscalls.json, misc/generated/subsystems.json

Which usually means that commands are depending on a file, instead of a target that wraps the file.

"You can’t just depend on gen1 instead of gen1-wrapper, because it may end up being built multiple times!"

https://samthursfield.wordpress.com/2015/11/21/cmake-dependencies-between-targets-and-files-and-custom-commands/

Copy link
Contributor

@SebastianBoe SebastianBoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dependencies need to be fixed.

@andrewboie
Copy link
Contributor Author

I was not able to reproduce the issue of make clean not working.

But I see that this happens:

[  1%] Preparing syscall dependency handling
[  1%] Preparing syscall dependency handling
[  2%] Preparing syscall dependency handling



[  3%] Generating misc/generated/syscalls_subdirs.trigger
[  4%] Generating misc/generated/syscalls_subdirs.trigger
[  5%] Generating misc/generated/syscalls_subdirs.trigger
[  6%] Generating misc/generated/syscalls.json, misc/generated/subsystems.json
[  7%] Generating misc/generated/syscalls.json, misc/generated/subsystems.json
[  7%] Generating misc/generated/syscalls.json, misc/generated/subsystems.json

Which usually means that commands are depending on a file, instead of a target that wraps the file.

"You can’t just depend on gen1 instead of gen1-wrapper, because it may end up being built multiple times!"

samthursfield.wordpress.com/2015/11/21/cmake-dependencies-between-targets-and-files-and-custom-commands

@SebastianBoe I can reproduce this without this PR.

For example, in master branch, I built, 'make clean', and 'make' again:

apboie@apboie-mobl4:~/projects/zephyr3/zephyr/tests/kernel/mem_protect/userspace/out master /home/apboie/projects/zephyr3/zephyr
$ mj
[  0%] Preparing syscall dependency handling
[  1%] Preparing syscall dependency handling
[  1%] Preparing syscall dependency handling



[  2%] Generating misc/generated/syscalls_subdirs.trigger
[  3%] Generating misc/generated/syscalls_subdirs.trigger
[  4%] Generating misc/generated/syscalls_subdirs.trigger
[  4%] Generating misc/generated/syscalls.json, misc/generated/subsystems.json
[  5%] Generating misc/generated/syscalls.json, misc/generated/subsystems.json
Traceback (most recent call last):
  File "/home/apboie/projects/zephyr3/zephyr/scripts/subfolder_list.py", line 80, in <module>
    main()
  File "/home/apboie/projects/zephyr3/zephyr/scripts/subfolder_list.py", line 56, in main
    os.symlink(directory, symlink)
FileExistsError: [Errno 17] File exists: '/home/apboie/projects/zephyr3/zephyr/include/dt-bindings/interrupt-controller' -> '/home/apboie/projects/zephyr3/zephyr/tests/kernel/mem_protect/userspace/out/zephyr/misc/generated/syscalls_links/_home_apboie_projects_zephyr3_zephyr_include_dt-bindings_interrupt-controller'
[  5%] Generating include/generated/driver-validation.h
[  6%] Generating include/generated/kobj-types-enum.h, include/generated/otype-to-str.h
zephyr/CMakeFiles/syscall_list_h_target.dir/build.make:166: recipe for target 'zephyr/misc/generated/syscalls_subdirs.trigger' failed
make[2]: *** [zephyr/misc/generated/syscalls_subdirs.trigger] Error 1
make[2]: *** Deleting file 'zephyr/misc/generated/syscalls_subdirs.trigger'
CMakeFiles/Makefile2:797: recipe for target 'zephyr/CMakeFiles/syscall_list_h_target.dir/all' failed
make[1]: *** [zephyr/CMakeFiles/syscall_list_h_target.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
[  6%] Built target driver_validation_h_target
[  6%] Built target kobj_types_h_target
Makefile:83: recipe for target 'all' failed
make: *** [all] Error 2

I think it is a race condition on whether the failure occurs, which is why I don't always see it.

Please reconsider your -1 as this issue is not related to this PR.

@andrewboie
Copy link
Contributor Author

I opened #23504 as the problems seen are unrelated to this PR.

@andrewboie
Copy link
Contributor Author

andrewboie commented Mar 16, 2020

@SebastianBoe I've root-caused these problems to #23183 it's not this patch.
Edit: I have a fix #23505

Copy link
Contributor

@SebastianBoe SebastianBoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing NACK

Andrew Boie added 8 commits March 17, 2020 09:50
Rather than stuffing various values in a uintptr_t based on
type using casts, use a union for this instead.

No functional difference, but the semantics of the data member
are now much clearer to the casual observer since it is now
formally defined by this union.

Signed-off-by: Andrew Boie <[email protected]>
Private data type, prefix with z_.

Signed-off-by: Andrew Boie <[email protected]>
Private structure, rename to z_object_assignment

Signed-off-by: Andrew Boie <[email protected]>
Private type, internal to the kernel, not directly associated
with any k_object_* APIs. Is the return value of z_object_find().
Rename to struct z_object.

Signed-off-by: Andrew Boie <[email protected]>
These were renamed to z_ prefix some time ago, but not updated
in these places.

Signed-off-by: Andrew Boie <[email protected]>
This never needed to be put in a separate gperf table.
Privilege mode stacks can be generated by the main
gen_kobject_list.py logic, which we do here.

Signed-off-by: Andrew Boie <[email protected]>
No need for this to be separated out any more.
Minimal changes made to get it to still work.

Signed-off-by: Andrew Boie <[email protected]>
Un-do the changes made to the code when this was separated out
to elf_helper.py.

Signed-off-by: Andrew Boie <[email protected]>
@andrewboie andrewboie merged commit b42fe9c into zephyrproject-rtos:master Mar 17, 2020
@andrewboie andrewboie deleted the userspace-cleanups branch March 17, 2020 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: API Changes to public APIs area: ARM ARM (32-bit) Architecture area: Build System area: Documentation area: Kernel area: Tests Issues related to a particular existing or missing test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

merge gen_kobject_list.py and gen_priv_stacks.py

6 participants