21 Dec
2021
21 Dec
'21
3:40 p.m.
On Wed, Dec 08, 2021 at 12:12:41PM +0100, Cezary Rojewski wrote:
+int avs_dsp_get_core(struct avs_dev *adev, u32 core_id) +{
...
- if (atomic_add_return(1, ref) == 1) {
ret = avs_dsp_enable(adev, mask);if (ret)goto err_enable_dsp;- }
+int avs_dsp_put_core(struct avs_dev *adev, u32 core_id) +{
...
- ref = &adev->core_refs[core_id];
- if (atomic_dec_and_test(ref)) {
ret = avs_dsp_disable(adev, mask);
This looks wrong - there's nothing that ensures that we don't get a sequence like:
CPU0 CPU1 decrement increment enable DSP disable DSP
that I can see here? Either there's a lock missing which ensures that the actual DSP management is in sync with the refcount or there's no need for the use of atomics since the wider lock will ensure that only one thing could be updating at once. In general I'd expect something heavier weight than atomics.