On 13 June 2017 at 09:12, Krzysztof Kozlowski <[email protected]> wrote: > On Mon, Jun 12, 2017 at 09:09:59PM +0200, Ulf Hansson wrote: >> On 12 June 2017 at 17:17, Krzysztof Kozlowski <[email protected]> wrote: >> > Add lockdep checks for holding mutex protecting the list of domains. >> > This might expose misuse even though only file-scope functions use it >> > for now. >> >> I think it seems a bit silly to use these lockdep checks as these >> functions are as you state above, static functions. Moreover there are >> called from a quite limited amount of places. >> >> Do you really think this add some value? > > In ideal world, these would not need lockdeps because we do not make > mistakes. I agree, that mostly the exposed functions to other kernel > modules should be protected, as lockdep annotation is a more advanced > way of documenting the need of locking. > > Does it mean that file-scope functions should not be annotated? Even in > such case function can be misused as the code is getting more and more > complicated. > > What is best example, one of these calls (pm_genpd_present()) was > already used in wrong way, without locking when iterating over the list. > Having lockdep would point this early.
Well, I think we found a potential bug because of reviewing the code. Not because of adding lockdep_assert_held(). Perhaps adding protection with lockdep_assert_held() could be meaningful also for static functions, but only in cases of complicated code. I don't think that is the case here. Anyway, if other people thinks $subject patch is good idea, then I drop my case. :-) [...] Kind regards Uffe
