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

pm: device_runtime: Optimize pm_device_runtime_usage #81453

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

nordic-krch
Copy link
Contributor

There is no point in using lock or semaphore to read current usage counter as it may change after unlocking or giving back the semaphore. Value can only be trusted in the controlled environment (e.g. test).

There is no point in using lock or semaphore to read current
usage counter as it may change after unlocking or giving
back the semaphore. Value can only be trusted in the controlled
environment (e.g. test).

Signed-off-by: Krzysztof Chruściński <[email protected]>
@bjarki-andreasen
Copy link
Collaborator

While I agree the count should only be used in controlled environment, it is also used in the device_service.c shell :) Could you add a commit removing the access and printing of usage count from device_service.c as well since use here requires the call to be "safe"?

@nordic-krch
Copy link
Contributor Author

Could you add a commit removing the access and printing of usage count

I'm not sure if I understand. Why no leave it as it is? Imo this does not change much for that command. pm_device_state_get and then pm_device_runtime_usage are get without any lock so it is possible that some device operation will end or start between and the won't match. I could maybe add lock around reading both then report would always be in sync.

@nashif
Copy link
Member

nashif commented Nov 26, 2024

@ceolin please review as this removed code you introduced.

@ceolin
Copy link
Member

ceolin commented Nov 26, 2024

I don't oppose to this change but I don't see much benefit as well. This function was introduced because of a shell functionality and it is used there and in a test (in the repo). The lock ensures the correctness of the count at the time it was called, obviously as soon as it is retrieved there is not guarantee that it will change.

@ceolin
Copy link
Member

ceolin commented Nov 26, 2024

Could you add a commit removing the access and printing of usage count

I'm not sure if I understand. Why no leave it as it is? Imo this does not change much for that command. pm_device_state_get and then pm_device_runtime_usage are get without any lock so it is possible that some device operation will end or start between and the won't match. I could maybe add lock around reading both then report would always be in sync.

I didn't get the point too. The shell is just informative and this change does not impact this functionality.

@nashif nashif merged commit 3b47ec6 into zephyrproject-rtos:main Nov 26, 2024
26 checks passed
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.

8 participants