cb92173d1f0 ("locking/lockdep, cpu/hotplug: Annotate AP thread") was
introduced to make lockdep_assert_cpus_held() work in AP thread.
However, the annotation is too strong for that purpose. We don't have to
use more than try lock annotation for that.
Furthermore, now that Dept was introduced, false positive alarms was
reported by that. Replaced it with try lock annotation.
Signed-off-by: Byungchul Park <[email protected]>
---
kernel/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index a86972a91991..b708989f789f 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -535,7 +535,7 @@ int lockdep_is_cpus_held(void)
static void lockdep_acquire_cpus_lock(void)
{
- rwsem_acquire(&cpu_hotplug_lock.dep_map, 0, 0, _THIS_IP_);
+ rwsem_acquire(&cpu_hotplug_lock.dep_map, 0, 1, _THIS_IP_);
}
static void lockdep_release_cpus_lock(void)
--
2.17.1
On Wed, Jan 24 2024 at 20:59, Byungchul Park wrote:
Why is lockdep in the subsystem prefix here? You are changing the CPU
hotplug (not hotplus) code, right?
> cb92173d1f0 ("locking/lockdep, cpu/hotplug: Annotate AP thread") was
> introduced to make lockdep_assert_cpus_held() work in AP thread.
>
> However, the annotation is too strong for that purpose. We don't have to
> use more than try lock annotation for that.
This lacks a proper explanation why this is too strong.
> Furthermore, now that Dept was introduced, false positive alarms was
> reported by that. Replaced it with try lock annotation.
I still have zero idea what this is about.
Thanks,
tglx
On Fri, Jan 26, 2024 at 06:30:02PM +0100, Thomas Gleixner wrote:
> On Wed, Jan 24 2024 at 20:59, Byungchul Park wrote:
>
> Why is lockdep in the subsystem prefix here? You are changing the CPU
> hotplug (not hotplus) code, right?
>
> > cb92173d1f0 ("locking/lockdep, cpu/hotplug: Annotate AP thread") was
> > introduced to make lockdep_assert_cpus_held() work in AP thread.
> >
> > However, the annotation is too strong for that purpose. We don't have to
> > use more than try lock annotation for that.
>
> This lacks a proper explanation why this is too strong.
>
> > Furthermore, now that Dept was introduced, false positive alarms was
> > reported by that. Replaced it with try lock annotation.
>
> I still have zero idea what this is about.
1. can track PG_locked that is a potential deadlock trigger.
https://lore.kernel.org/lkml/[email protected]/
2. can track any waits/events e.g. wait_for_xxx(), dma fence and so on.
3. easy to annotate using dept_wait() on waits, dept_event() on events.
4. track read lock better way instead of the ugly way, by assinging wait
or event annotations onto read lock and write lock. For instrance, a
read lock is annotated as a potential waiter for its write unlock,
and a write lock is annotated as a potential waiter for either write
unlock or read unlock.
I'd like to remove unnecessary complexity on deadlock detection and add
additional functionality by making it do what the type of tool exactly
should do.
Byungchul
> Thanks,
>
> tglx
On Tue, Jan 30 2024 at 11:58, Byungchul Park wrote:
> On Fri, Jan 26, 2024 at 06:30:02PM +0100, Thomas Gleixner wrote:
>> On Wed, Jan 24 2024 at 20:59, Byungchul Park wrote:
>>
>> Why is lockdep in the subsystem prefix here? You are changing the CPU
>> hotplug (not hotplus) code, right?
>>
>> > cb92173d1f0 ("locking/lockdep, cpu/hotplug: Annotate AP thread") was
>> > introduced to make lockdep_assert_cpus_held() work in AP thread.
>> >
>> > However, the annotation is too strong for that purpose. We don't have to
>> > use more than try lock annotation for that.
>>
>> This lacks a proper explanation why this is too strong.
>>
>> > Furthermore, now that Dept was introduced, false positive alarms was
>> > reported by that. Replaced it with try lock annotation.
>>
>> I still have zero idea what this is about.
>
> 1. can track PG_locked that is a potential deadlock trigger.
>
> https://lore.kernel.org/lkml/[email protected]/
Sure, but that wants to be explicitely explained in the changelog and
not with a link. 'Now that Dept was introduced ...' is not an
explanation.
Thanks,
tglx
On Fri, Jan 26, 2024 at 06:30:02PM +0100, Thomas Gleixner wrote:
> > Furthermore, now that Dept was introduced, false positive alarms was
> > reported by that. Replaced it with try lock annotation.
>
> I still have zero idea what this is about.
Lockdep is working on lock/unlock, while dept is working on wait/event.
Two are similar but strickly speaking, different in what to track.
Byungchul