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 <[email protected]> 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. > > >
_______________________________________________ Kernelnewbies mailing list [email protected] https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
