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

High CPU usage on new SPI work queue threads #13149

Closed
JacobCrabill opened this issue Oct 9, 2019 · 13 comments
Closed

High CPU usage on new SPI work queue threads #13149

JacobCrabill opened this issue Oct 9, 2019 · 13 comments
Labels

Comments

@JacobCrabill
Copy link
Member

JacobCrabill commented Oct 9, 2019

I'm not sure this is a bug per se, but @TSC21 thought it was worth bringing up.

Describe the bug
In the 1.10.0-beta2 tag, the CPU usage of the SPI work queues (SPI1 and SPI4) are extremely high, and the remaining/idle CPU is worryingly low. This is on a standard CubeBlack (fmu-v3) with a standard Proficnc carrier board. See the attached images for a comparison of armed/disarmed CPU usage for 1.9.2 vs. 1.10.0 for the same Cube.

To Reproduce
Steps to reproduce the behavior:

  1. CubeBlack on carrier board, nothing but micro-USB plugged in
  2. Load PX4 v1.9.2 or PX4 v1.10.0-beta2 firmware for fmu-v3
  3. Set SYS_AUTOSTART to 13000 (generic standard VTOL) and SYS_AUTOCONFIG to 1
  4. Run top and compare CPU usage

Expected behavior
In v1.9.2, the overall CPU usage is around 50-60%. In 1.10.0-beta2, the usage is 95% when disarmed, 99% when armed.

We've flown this on an X8 VTOL as well (just MC hover tests in stabilized and altitude modes). I can upload logs for those if desired; the CPU usages in those cases were essentially the same as the bench test.

Log Files and Screenshots
v1.9.2, armed
PX4-1 9 2_top-armed

v1.9.2, disarmed
PX4-1 9 2_top-disarmed

v1.10.0-beta2, armed
PX4-1 10 0-beta2_top-armed

v1.10.0-beta2, disarmed
PX4-1 10 0-beta2_top-disarmed

Drone (please complete the following information):

  • CubeBlack + carrier board
  • Generic or custom VTOL airframe config
@julianoes
Copy link
Contributor

Thanks for testing and reporting this!

As I know from @dagar the recent changes to the work queues make CPU usage used by SPI transfers visible which were hidden beforehand. This means that it is expected that CPU usage will increase, however, performance should get better or at least stay the same.

However, if CPU usage is above > 90% as you describe that's certainly something to look at. I'll wait for @dagar to comment on the specifics.

@LorenzMeier
Copy link
Member

I think the issue here is configuration: The rate controller also is running here much, much faster. @julianoes @dagar we need to make sure the new defaults (which are great for racers) are not causing issues for regular setups.

@JacobCrabill
Copy link
Member Author

I figured this was related to the work being done to allow the IMUs / rate controllers to be run much faster. We're planning to use this on a rather large VTOL platform, so definitely a different end of the spectrum than a race quad!

This could also contribute to logging dropout, correct?

Are these defaults something that could be configured by the airframe? For example, #13074 is an attempt to create generic size-based airframes; some documentation on parameters to control the rate controller / IMU driver rates and tips/guidance on selecting those values (based on the rough size & type of airframe) could be a decent solution.

I've uploaded a log from one of our test flights here: https://review.px4.io/plot_app?log=909e40ba-17b0-4ded-bc88-49a48aa37b8f
You can see that the CPU usage is around a constant 90%, with short dips that also correspond to logging dropouts.

@dagar
Copy link
Member

dagar commented Oct 11, 2019

@JacobCrabill you can set IMU_GYRO_RATEMAX 400 Hz to match the PWM rate (guessing for your setup) or 250 Hz for the old behavior.

I've considered making this the default for some cases, but so far it doesn't quite fit into the existing "categories" of things we carry different defaults for (board type, vehicle type, etc).

In previous PX4 releases many of the drivers ran out of interrupt service routines (ISRs) where they not only transferred the data, but also performed filtering and integration before publishing to the rest of the system. This time spent in ISRs is hidden in top, slightly accounted for in other tasks, but is largely happening in your "Idle Task". So unfortunately that 40-50% idle you saw previously was a bit of an illusion.
Moving all these relatively high rate drivers (> 1 kHz) from ISRs to work queue threads costs a few percent more in cpu due to context switches and SPI locking (we can get rid of eventually), but it's roughly the same amount of time spent in each.

@dagar
Copy link
Member

dagar commented Oct 11, 2019

The 10% cpu usage in mc_pos_control is also something to investigate.

@dagar
Copy link
Member

dagar commented Oct 15, 2019

For VTOL we can limit IMU_GYRO_RATEMAX as a precaution for now. #13196

@JacobCrabill
Copy link
Member Author

Haven't had a chance to do any flight testing to look at this specifically, but on a bench test reducing IMU_GYRO_RATEMAX to 400 did noticeably reduce the CPU load.

Good to know about the ISRs - I never knew that top could be missing out on 'hidden' work.

Do we have some flexibility in running these drivers at a lower rate if we decide to do so? I.e., ignoring any issues from a control theory standpoint, are there any assumptions made now in the driver / module architecture that depend on these drivers being run at 1kHz or more?

dagar added a commit that referenced this issue Oct 16, 2019
@JacobCrabill
Copy link
Member Author

Did a flight yesterday with the (at the time) latest version of master (cefa7ec); prior to changing IMU_GYRO_RATEMAX from the original value of 0 we were hovering between 95-100% CPU usage (using MC altitude and position modes); after changing to 400Hz, CPU usage dropped by at least 10% and we experienced far fewer logging dropouts. I feel comfortable using this patch in our vehicles until we transition to a newer hardware standard.

@proficnc
Copy link

This is skirting too close to the edge. Time to push cube orange

@JacobCrabill
Copy link
Member Author

@proficnc Are the new STM32H7 processors fully supported in PX4? I thought I saw mention of some support missing recently. Also, what's the country of origin for Cube Orange?

@dagar
Copy link
Member

dagar commented Oct 21, 2019

Another small fix last week was an unnecessary double promotion in FlightTasks.

e942d3a

@julianoes
Copy link
Contributor

@dagar would you agree that we can close this issue then? Of course, this doesn't mean that we want to further reduce CPU usage over time but at least for the release we can argue it's good enough and probably better than in the last release where it wasn't reported correctly.

@dagar
Copy link
Member

dagar commented Oct 22, 2019

Yes I think we can close this. It's mainly a potential issue for the more complicated setups on older boards, and in the short term those cases can be remedied via parameter (IMU_GYRO_RATEMAX, killing unnecessary mavlink instances or reducing the data rate, etc). The other thing to keep in mind is that we've been fairly careful structuring the work queues, tasks, and priorities such that the sensor sampling, controllers, and estimators will continue to function safely even on a fully loaded system. There will be more savings coming soon after that next release with DMA for SPI IMUs and serial TX (mavlink).

@dagar dagar closed this as completed Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants