From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id A44D7C77B6E for ; Thu, 13 Apr 2023 14:49:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 075E2900002; Thu, 13 Apr 2023 10:49:56 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F405E6B0074; Thu, 13 Apr 2023 10:49:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DBA5E900002; Thu, 13 Apr 2023 10:49:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id C8B1F6B0072 for ; Thu, 13 Apr 2023 10:49:55 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 8E50FAB0A0 for ; Thu, 13 Apr 2023 14:49:55 +0000 (UTC) X-FDA: 80676652350.16.10F76B2 Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) by imf23.hostedemail.com (Postfix) with ESMTP id 3C0FC140011 for ; Thu, 13 Apr 2023 14:49:52 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=fjnRyzyl; dmarc=pass (policy=quarantine) header.from=bytedance.com; spf=pass (imf23.hostedemail.com: domain of zhengqi.arch@bytedance.com designates 209.85.214.173 as permitted sender) smtp.mailfrom=zhengqi.arch@bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1681397393; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Muyh3IamnE6ZXDdZVJY7OPyQrN7OXiGiKOYyXgJGepo=; b=TALCS19UuDD4N+Jo2t6wFT55jEJkU/LBFGnkv2JaaFOYu8eL55r+SetJORFsSC6DPNJVX9 U57Wt5rBvaamw3oS6Tg7XSYdh0r4gB8b7KKGpq0RicS9SjIb9J42K3BGqgvzjarT45kOe1 Ic49IsmjpNctuFXdn1QNKIck8wcJww8= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=fjnRyzyl; dmarc=pass (policy=quarantine) header.from=bytedance.com; spf=pass (imf23.hostedemail.com: domain of zhengqi.arch@bytedance.com designates 209.85.214.173 as permitted sender) smtp.mailfrom=zhengqi.arch@bytedance.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681397393; a=rsa-sha256; cv=none; b=cAW4EukSZjz1qixKhbuFiBrXif+xmujMe8L7dG35JSLZ8UAkbUHB1VcC0W0IHbpPD3gP50 sVEKMLSMKe+kVhV4RqVgCWQb9/tmX/51GoB7P4en0kthbYiuxxKjWKv5fgE5+NtELdiLi4 jvqcDlXzI/amBLHj/tji41GZYFkFapY= Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-1a4f8e5d190so6741255ad.1 for ; Thu, 13 Apr 2023 07:49:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance.com; s=google; t=1681397391; x=1683989391; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=Muyh3IamnE6ZXDdZVJY7OPyQrN7OXiGiKOYyXgJGepo=; b=fjnRyzylBZKtO3ZmkaMWPg3zH8+StLYzJbPcaouyG2InQdEBl+f98v5P2uwE+Lp+vf QOAX0rjfrlg560sCfarppCv3qTJSdS3nNP4Jm7BqUV6FeZzpOMatS8gQylnsAlglyl5Y pfO0UaoE6MwEaZJLoehzRVqpFC0liewCjsSw4UxX/45Px8PP3jFSrbkoN2ZtmuNM8ezU cVBxbnQSnY7en6Un7ANRr+DA91wuPCCqw/hjvwW/u3PazyXiCZO+5r1UwJ6SGv14CMgW bppOv+6seAwcOBKNJSMpK2r+6uyYTPKhHIo0cpaoSVLtxSAwNxrsKV8xs5A2t+jY3THn xgYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681397391; x=1683989391; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Muyh3IamnE6ZXDdZVJY7OPyQrN7OXiGiKOYyXgJGepo=; b=ND69vGYyNHXDRjtRbo1Lz9t7h0pxTnjYIh4R9d/zkPVz+oU7afwTegOxMPxYkR+vto Gn8Rt223m7/oW5WBmDkJZv2VNU1oCVXriaHBYwaikEF6xzRygOjo5coc/yn+DUZjLwN5 ayJcnI/6whvgm7d68fva2aUC//5QZ8Ot+SCNMH/kCw+TooMi3Tz6jW+VvG5HW90XV67M QQk8Fln8iEsPl1YRQtkLAPO2UMUgFjavoPeODcFiy6852kTFwlqfLJemmbnyq3MAmi9Y rQR5l2CzFyfwKcL25vT43ndeeFdl6rqv3YfWKvBNAIyaHEG02tn3ocZlTjUN41d7lYcz bjTw== X-Gm-Message-State: AAQBX9fQZHUWL/XnOl9ndWl37R0mfmZM7AqLWrSClScVftSYJ9LE4RA2 TdxsFmbbaydbEKDf7GBptZ+WQQ== X-Google-Smtp-Source: AKy350ZuESBw6VXWyAMZYgNqo7cuyPBDfOGHxCLXaVZ1+aPQGZJqQlgsGmjF4s98yYZBorJ2lir/2w== X-Received: by 2002:a17:90a:1954:b0:246:aedc:496 with SMTP id 20-20020a17090a195400b00246aedc0496mr2003572pjh.2.1681397390630; Thu, 13 Apr 2023 07:49:50 -0700 (PDT) Received: from [10.200.11.168] ([139.177.225.254]) by smtp.gmail.com with ESMTPSA id p15-20020a17090a2d8f00b00246626343aesm3288291pjd.25.2023.04.13.07.49.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 13 Apr 2023 07:49:50 -0700 (PDT) Message-ID: <162e6281-8828-e0bc-2b91-183b7f3a1f65@bytedance.com> Date: Thu, 13 Apr 2023 22:49:41 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.9.1 Subject: Re: [PATCH] mm: slub: annotate kmem_cache_node->list_lock as raw_spinlock Content-Language: en-US To: Peter Zijlstra Cc: Vlastimil Babka , "Zhang, Qiang1" , Boqun Feng , "42.hyeyoo@gmail.com" <42.hyeyoo@gmail.com>, "akpm@linux-foundation.org" , "roman.gushchin@linux.dev" , "iamjoonsoo.kim@lge.com" , "rientjes@google.com" , "penberg@kernel.org" , "cl@linux.com" , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , Zhao Gongyi , Sebastian Andrzej Siewior , Thomas Gleixner , RCU , "Paul E . McKenney" References: <20230411130854.46795-1-zhengqi.arch@bytedance.com> <932bf921-a076-e166-4f95-1adb24d544cf@bytedance.com> <20230412124735.GE628377@hirez.programming.kicks-ass.net> <76e15f10-d3f1-2cab-63cb-25aa3b4f2cd4@bytedance.com> From: Qi Zheng In-Reply-To: <76e15f10-d3f1-2cab-63cb-25aa3b4f2cd4@bytedance.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 3C0FC140011 X-Stat-Signature: b8udz3bqp1grr4rbp8ibkndw453pqy9y X-HE-Tag: 1681397392-981239 X-HE-Meta: U2FsdGVkX19+7e3U/ZEpWq4PSJgHo1oTZnvKd5UNYJJFds0mhOgxvZpjDNRTo9cLdefIZVC7fbLAJKnnvRC1Rd/XodJDfmEiNfMqAV1ihsvk4AcRFuo5z5LCEsAQuBWwB6wFCjUtGx3fZrIjn8FdTqa5FjBsELjJCoKkxsnIvl/e07xZQl8XcA4IMjCb5H7SH7A9VI4EOcd8XTef1jXPe+NoVEKjLj2ZLVxEp4wI0ssCcPE8F8O3zI9uoYUmu/3AUngJgmaTogR0PlVtNIM0OrMzfUVzWZLidDJdQejZty/jn70FaLRxx6R4Qs8wdFwxEDa77jI9S4edlhPPSoPUk0mlXw8YSKijKXPQIbVlr3SYlxSqh62D2qDIqYbWh9WPCg4QVewo1UFbDK8xg58Gzj2DRkzTROTLTcMudDrJxLpm6KLGHIo28bJGKnlaS9SP6N+DZmE8Ml3wgBP5D+1N7Dn+wheo1gWQK2rDCx6zenOraYgBADdcdpboZ+O/aCV8k4lSJc9Ep3nUWgTEEzSQPOfbxTcJKd64DybsyWQ0DNEjp2qN8VH3tS0mb4Ggqjlqm8ocgEipcC2ot9Y4pSR5rm8ixbrxzZpIdVZ20S1UAOnwoAyoKYPI27ATPehAjDco9ia9biTOIz4NnUe+9mk6Jp7gqmvaPwu+WiNUElUEMpY2NNyXIk1G/NMM1sFTCVjXnW5WetPjCyK7ohwqLfslrI9eIDzE6NIm3MjjYt4WH925DB8QacBLE2kCMRzRvVLEmcQKAkzewdnzP+EV7wnGBhxJQ/AhqRSKbrLmCAU1Sp/YmQEmgQXaXBI2FuS1OhP/qlM67BDSSyXBDBvT5+UIZh6euq4rOvFVzerycoGUnKviiIeH4TiU+tIJXkDQb8oJcA6FXxNwGUldb7DzugvUdVYoNTU/yS3hzTd0p6unnmFDtGfUAZlIAAyvvMKL6J0wLEtXe0c/OZFzqlBmOXo 7nMSxsCz LPc3OYs5/uRSYubjx2WvR8d3MtVJj/0nbhWxiLlCYoHEwyq4Zx5WbzjOKa9kBerdMYXg70qgwnLzoQlTdswJm/6f/ZPcbdZ/QnuYDLxu04tAJHlL/THoiJwwUITDQqQxvtDasARIGxqxomiRjUz/j093bDsPbbzN3BGoBiuXeuP+UknEMQW+kCJ1MmR+eqIoEZuUZhh7hvPKQ/b2uRMWcu7WM2N/RPTNskrpkoZWHFOAhgcEkPmR8IiZBeEUrnW4Np7UWKuxXNKHOoyr1bkK1JfM4ot7/HIQypEF/BGYymnRnmE9bLYLPiKSsHVUegkWcKUmSUQAq3Id6x1IW0kBpmzBcMGrTfKfKumPTaMC5DA3mpQkYMAyWVpwhHndJOYC+YrkCFGqKvcXMWewvxn4DMCbr05zAejzENFlVRn2Oe0EX+gc56IJ+071nfu3Vuvvj8nK7LvXVlNHhnWE= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 2023/4/13 00:44, Qi Zheng wrote: > > > On 2023/4/12 20:47, Peter Zijlstra wrote: >> On Wed, Apr 12, 2023 at 08:50:29AM +0200, Vlastimil Babka wrote: >> >>>> --- a/lib/debugobjects.c >>>> +++ b/lib/debugobjects.c >>>> @@ -562,10 +562,10 @@ __debug_object_init(void *addr, const struct >>>> debug_obj_descr *descr, int onstack >>>>          unsigned long flags; >>>> >>>>          /* >>>> -        * On RT enabled kernels the pool refill must happen in >>>> preemptible >>>> +        * The pool refill must happen in preemptible >>>>           * context: >>>>           */ >>>> -       if (!IS_ENABLED(CONFIG_PREEMPT_RT) || preemptible()) >>>> +       if (preemptible()) >>>>                  fill_pool(); >>> >>> +CC Peterz >>> >>> Aha so this is in fact another case where the code is written with >>> actual differences between PREEMPT_RT and !PREEMPT_RT in mind, but >>> CONFIG_PROVE_RAW_LOCK_NESTING always assumes PREEMPT_RT? >> >> Ooh, tricky, yes. PROVE_RAW_LOCK_NESTING always follows the PREEMP_RT >> rules and does not expect trickery like the above. >> >> Something like the completely untested below might be of help.. >> >> --- >> diff --git a/include/linux/lockdep_types.h >> b/include/linux/lockdep_types.h >> index d22430840b53..f3120d6a7d9e 100644 >> --- a/include/linux/lockdep_types.h >> +++ b/include/linux/lockdep_types.h >> @@ -33,6 +33,7 @@ enum lockdep_wait_type { >>   enum lockdep_lock_type { >>       LD_LOCK_NORMAL = 0,    /* normal, catch all */ >>       LD_LOCK_PERCPU,        /* percpu */ >> +    LD_LOCK_WAIT,        /* annotation */ >>       LD_LOCK_MAX, >>   }; >> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c >> index 50d4863974e7..a4077f5bb75b 100644 >> --- a/kernel/locking/lockdep.c >> +++ b/kernel/locking/lockdep.c >> @@ -2279,8 +2279,9 @@ static inline bool usage_skip(struct lock_list >> *entry, void *mask) >>        * As a result, we will skip local_lock(), when we search for irq >>        * inversion bugs. >>        */ >> -    if (entry->class->lock_type == LD_LOCK_PERCPU) { >> -        if (DEBUG_LOCKS_WARN_ON(entry->class->wait_type_inner < >> LD_WAIT_CONFIG)) >> +    if (entry->class->lock_type != LD_LOCK_NORMAL) { >> +        if (entry->class->lock_type == LD_LOCK_PERCPU && >> +            DEBUG_LOCKS_WARN_ON(entry->class->wait_type_inner < >> LD_WAIT_CONFIG)) >>               return false; >>           return true; >> @@ -4752,7 +4753,8 @@ static int check_wait_context(struct task_struct >> *curr, struct held_lock *next) >>       for (; depth < curr->lockdep_depth; depth++) { >>           struct held_lock *prev = curr->held_locks + depth; >> -        u8 prev_inner = hlock_class(prev)->wait_type_inner; >> +        struct lock_class *class = hlock_class(prev); >> +        u8 prev_inner = class->wait_type_inner; >>           if (prev_inner) { >>               /* >> @@ -4762,6 +4764,12 @@ static int check_wait_context(struct >> task_struct *curr, struct held_lock *next) >>                * Also due to trylocks. >>                */ >>               curr_inner = min(curr_inner, prev_inner); >> + >> +            /* >> +             * Allow override for annotations. >> +             */ >> +            if (unlikely(class->lock_type == LD_LOCK_WAIT)) >> +                curr_inner = prev_inner; >>           } >>       } >> diff --git a/lib/debugobjects.c b/lib/debugobjects.c >> index df86e649d8be..fae71ef72a16 100644 >> --- a/lib/debugobjects.c >> +++ b/lib/debugobjects.c >> @@ -565,8 +565,16 @@ __debug_object_init(void *addr, const struct >> debug_obj_descr *descr, int onstack >>        * On RT enabled kernels the pool refill must happen in preemptible >>        * context: >>        */ >> -    if (!IS_ENABLED(CONFIG_PREEMPT_RT) || preemptible()) >> +    if (!IS_ENABLED(CONFIG_PREEMPT_RT) || preemptible()) { >> +        static lockdep_map dep_map = { > >                 static struct lockdep_map dep_map = { > >> +            .name = "wait-type-override", >> +            .wait_type_inner = LD_WAIT_SLEEP, >> +            .lock_type = LD_LOCK_WAIT, >> +        }; >> +        lock_map_acquire(&dep_map); >>           fill_pool(); >> +        lock_map_release(&dep_map); >> +    } >>       db = get_bucket((unsigned long) addr); > > I just tested the above code, and then got the following > warning: > > > It seems that the LD_WAIT_SLEEP we set is already greater than the > LD_WAIT_SPIN of the current context. > Can we do something like below? This solves the warning I encountered. diff --git a/include/linux/lockdep_types.h b/include/linux/lockdep_types.h index d22430840b53..f3120d6a7d9e 100644 --- a/include/linux/lockdep_types.h +++ b/include/linux/lockdep_types.h @@ -33,6 +33,7 @@ enum lockdep_wait_type { enum lockdep_lock_type { LD_LOCK_NORMAL = 0, /* normal, catch all */ LD_LOCK_PERCPU, /* percpu */ + LD_LOCK_WAIT, /* annotation */ LD_LOCK_MAX, }; diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index dcd1d5bfc1e0..6859dba8a3aa 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -2289,8 +2289,9 @@ static inline bool usage_skip(struct lock_list *entry, void *mask) * As a result, we will skip local_lock(), when we search for irq * inversion bugs. */ - if (entry->class->lock_type == LD_LOCK_PERCPU) { - if (DEBUG_LOCKS_WARN_ON(entry->class->wait_type_inner < LD_WAIT_CONFIG)) + if (entry->class->lock_type != LD_LOCK_NORMAL) { + if (entry->class->lock_type == LD_LOCK_PERCPU && + DEBUG_LOCKS_WARN_ON(entry->class->wait_type_inner < LD_WAIT_CONFIG)) return false; return true; @@ -3981,6 +3982,9 @@ static inline int valid_state(struct task_struct *curr, struct held_lock *this, enum lock_usage_bit new_bit, enum lock_usage_bit bad_bit) { + if (unlikely(hlock_class(this)->lock_type == LD_LOCK_WAIT)) + return 1; + if (unlikely(hlock_class(this)->usage_mask & (1 << bad_bit))) { graph_unlock(); print_usage_bug(curr, this, bad_bit, new_bit); @@ -4768,7 +4772,8 @@ static int check_wait_context(struct task_struct *curr, struct held_lock *next) for (; depth < curr->lockdep_depth; depth++) { struct held_lock *prev = curr->held_locks + depth; - u8 prev_inner = hlock_class(prev)->wait_type_inner; + struct lock_class *class = hlock_class(prev); + u8 prev_inner = class->wait_type_inner; if (prev_inner) { /* @@ -4778,9 +4783,19 @@ static int check_wait_context(struct task_struct *curr, struct held_lock *next) * Also due to trylocks. */ curr_inner = min(curr_inner, prev_inner); + + /* + * Allow override for annotations. + */ + if (unlikely(class->lock_type == LD_LOCK_WAIT)) + curr_inner = prev_inner; } } + if (unlikely(hlock_class(next)->lock_type == LD_LOCK_WAIT && + curr_inner == LD_WAIT_SPIN)) + curr_inner = LD_WAIT_CONFIG; + if (next_outer > curr_inner) return print_lock_invalid_wait_context(curr, next); diff --git a/lib/debugobjects.c b/lib/debugobjects.c index df86e649d8be..a8a69991b0d0 100644 --- a/lib/debugobjects.c +++ b/lib/debugobjects.c @@ -565,8 +565,16 @@ __debug_object_init(void *addr, const struct debug_obj_descr *descr, int onstack * On RT enabled kernels the pool refill must happen in preemptible * context: */ - if (!IS_ENABLED(CONFIG_PREEMPT_RT) || preemptible()) + if (!IS_ENABLED(CONFIG_PREEMPT_RT) || preemptible()) { + static struct lockdep_map dep_map = { + .name = "wait-type-override", + .wait_type_inner = LD_WAIT_CONFIG, + .lock_type = LD_LOCK_WAIT, + }; + lock_map_acquire(&dep_map); fill_pool(); + lock_map_release(&dep_map); + } db = get_bucket((unsigned long) addr); -- Thanks, Qi