Skip to content
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

PX4 workqueue add simple reference counting to shutdown empty queues #12154

Closed
wants to merge 1 commit into from

Conversation

dagar
Copy link
Member

@dagar dagar commented Jun 3, 2019

When probing optional drivers at startup the corresponding workqueue per bus is created if it doesn't already exist. With the current init structure of attempting to start many possible external devices the result is a workqueue for every bus in the system. This doesn't actually waste that much memory, but we are starting to bump up against the NuttX task limit. #11792 (comment)

The downside of this PR is that in some cases the WQ for the same bus might be created, killed, then created again later as each optional driver is probing.

Example

INFO [px4_work_queue] creating: wq:I2C1, priority: 248, stack: 1200 bytes
WARN [bst] no devices found
INFO [px4_work_queue] wq:I2C1: last active WorkItem closing, stopping thread
INFO [px4_work_queue] wq:I2C1: exiting

@dagar
Copy link
Member Author

dagar commented Jun 3, 2019

Catkin CI failure might be real.

@dagar dagar force-pushed the pr-wq_ref_count branch from 81e011a to 3beb56c Compare June 4, 2019 14:47
@dagar
Copy link
Member Author

dagar commented Jun 4, 2019

TODO: go back through the WQ manager to initiate shutdown.

@dagar dagar changed the title PX4 workqueue add simple reference counting to shutdown empty queues [WIP] PX4 workqueue add simple reference counting to shutdown empty queues Jun 4, 2019
@dagar dagar force-pushed the pr-wq_ref_count branch 2 times, most recently from fb653d1 to 871415c Compare June 4, 2019 18:17
@dagar
Copy link
Member Author

dagar commented Jun 4, 2019

New idea

  • keep a count of active WorkItems per WorkQueue
  • work queue manager add cleanup function to shutdown WorkQueues without any WorkItems
  • add wq_manager command to call status and cleanup
  • at end of boot call wq_manager cleanup (similar to mavlink boot_complete)

cleanup (end of init)

INFO  [px4_work_queue] wq:SPI4 empty, stopping
INFO  [px4_work_queue] wq:SPI4: exiting
INFO  [px4_work_queue] wq:I2C1 empty, stopping
INFO  [px4_work_queue] wq:I2C1: exiting

status

nsh> wq_manager status
INFO  [px4_work_queue] wq:SPI2 running, priority (relative to max): -2, stack: 1200 bytes, open: 1
INFO  [px4_work_queue] wq:SPI1 running, priority (relative to max): -1, stack: 1200 bytes, open: 3
INFO  [px4_work_queue] wq:hp_default running, priority (relative to max): -11, stack: 1200 bytes, open: 2
INFO  [px4_work_queue] wq:lp_default running, priority (relative to max): -50, stack: 1200 bytes, open: 1

@dagar dagar changed the title [WIP] PX4 workqueue add simple reference counting to shutdown empty queues PX4 workqueue add simple reference counting to shutdown empty queues Jun 4, 2019
@davids5
Copy link
Member

davids5 commented Jun 4, 2019

New idea

  • keep a count of active WorkItems per WorkQueue
  • work queue manager add cleanup function to shutdown WorkQueues without any WorkItems
  • add wq_manager command to call status and cleanup
  • at end of boot call wq_manager cleanup (similar to mavlink boot_complete)

cleanup (end of init)

INFO  [px4_work_queue] wq:SPI4 empty, stopping
INFO  [px4_work_queue] wq:SPI4: exiting
INFO  [px4_work_queue] wq:I2C1 empty, stopping
INFO  [px4_work_queue] wq:I2C1: exiting

status

nsh> wq_manager status
INFO  [px4_work_queue] wq:SPI2 running, priority (relative to max): -2, stack: 1200 bytes, open: 1
INFO  [px4_work_queue] wq:SPI1 running, priority (relative to max): -1, stack: 1200 bytes, open: 3
INFO  [px4_work_queue] wq:hp_default running, priority (relative to max): -11, stack: 1200 bytes, open: 2
INFO  [px4_work_queue] wq:lp_default running, priority (relative to max): -50, stack: 1200 bytes, open: 1

I am concerned with fragmentation.

@dagar dagar force-pushed the pr-wq_ref_count branch from 871415c to 8c45b4b Compare June 6, 2019 01:29
@mcsauder
Copy link
Contributor

mcsauder commented Jun 8, 2019

I haven't reviewed these changes, but here is a flight log from this PR:
https://review.px4.io/plot_app?log=80ee6b6e-abaa-468b-b7a1-06798555c5b4

@bkueng
Copy link
Member

bkueng commented Jun 11, 2019

We can also get rid of the wq:manager thread which is only idling after bootup, and move thread start/stop handling into one of the wq that is running anyway.

@dagar
Copy link
Member Author

dagar commented Aug 31, 2019

Continued in #12853.

@dagar dagar closed this Aug 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants