Thanks a lot Valdis. These are very helpful pointers to look into. Regards, Ankit On Wed, Feb 9, 2022 at 10:06 PM Valdis Klētnieks <valdis.kletnieks@vt.edu> wrote:
On Tue, 08 Feb 2022 23:26:54 +0530, Ankit Pandey said:
And I was asked to verify if there is some specific meaning is attached to comment here (which was causing the issue). I would be glad you could explain me how should I approach this issue? One way would be to rewrite that these variables could be defined as volatile (just add a comment) and then compile driver and see that build goes through without any error.
It turns out that the C keyword 'volatile' usually doesn't actually do what needs to happen if a variable actually *is* volatile and subject to change while the executing thread isn't looking.
There's a good documentation file on this:
Documentation/process/volatile-considered-harmful.rst
But in summary - "If you thought you needed 'volatile' in your code, you probably needed locking primitives instead".
Other way would be that try to understand what this function is supposed to be doing and then figure out author's intent of putting volatile there. How should I take decision on these (or if they are wrong approaches) ?
Given that struct pwrctlr_priv already contains a mutex_lock, what was probably *intended* was "the variables cpwm, tog, cpwm_tog, and tgt_rpwm are protected by the mutex_lock and may only be changed by the mutex holder, while pwr_mode, smart_ps, and alives are not subject to change on the fly".
But actually reading and understanding the code would be required to verify that.