On Fri, Mar 20, 2020 at 04:39:48PM +0000, Mark Brown wrote:

On Fri, Mar 20, 2020 at 04:07:27PM +0100, Guennadi Liakhovetski wrote:

...

On Fri, Mar 20, 2020 at 02:27:32PM +0100, Guennadi Liakhovetski wrote:

...

...

No, this isn't a completion - it's a counter. I've used atomic variables before, I cannot remember seeing any difficulties with their correct use described. Do you have a pointer?

...

Actually I'd even say this isn't a problem. I think it's safe to say, you won't suspend and resume your audio interface more often than every 10 seconds. That makes under 3200000 cycles per year. Even with 31 bits for a signed integer that makes more than 600 years. I think we're safe.

The problem is that atomics are just incredibly error prone - for example they're just a plain number, they're usually counting something which is not being locked so you have to be careful any time you do anything around them. Their lack of structure makes them harder to reason about than most other locking primitives, often harder than it's worth.

Ok, understand, but do you see any problems with this specific use case? Thinking about possible replacements - it isn't a case for a ref-count, because it isn't a get-put scenario. We really just need to count a specific event - DSP reboots. It can be the case that this counter doesn't need to be atomic at all. When it is read, the DSP is guaranteed to be up and running - I think. So no race would be possible. I can try to think about this more carefully and maybe make it a normal unprotected counter. But I don't think it has to be protected even better than what these patches are doing.

Thanks Guennadi