lkml.org 
[lkml]   [2017]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2 3/8] PM / Domains: Add lockdep asserts for domains list mutex
On 13 June 2017 at 09:12, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Mon, Jun 12, 2017 at 09:09:59PM +0200, Ulf Hansson wrote:
>> On 12 June 2017 at 17:17, Krzysztof Kozlowski <krzk@kernel.org> 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

\
 
 \ /
  Last update: 2017-06-13 10:36    [W:0.075 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site