From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f176.google.com (mail-pf1-f176.google.com [209.85.210.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4F7BD1114 for ; Fri, 8 Sep 2023 20:52:08 +0000 (UTC) Received: by mail-pf1-f176.google.com with SMTP id d2e1a72fcca58-68a42d06d02so2220940b3a.0 for ; Fri, 08 Sep 2023 13:52:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1694206327; x=1694811127; darn=lists.linux.dev; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :sender:from:to:cc:subject:date:message-id:reply-to; bh=VDbHt9ZjxbWg0OnwI6q2vK5uaDqhBGdPDoTGYH2kvhs=; b=IRsSbrcxSTxiQfWG3HZUZgIk5Sxt0opg8M4ThyX5MlnC7j+r8zaB/7gCxPxcC0wCNp QoH15bf/wZL5OJ2tcjocHFgC7+kxsynunK1HFSWIRqy31dWRUnMhhR17xEaVNAqGxKMU JGfTRSmsohvxoVoP9/XME/qL8MbvVrldNG6FSas5hMRHrmDwqBr44uhGuyNz93ZkayNt x2n3a7wlh3Pg3bV9EB0t2PvSYtqBcMB9typ8UxYFUT3vhkYpcuuhXPOvvfCKmMegrsYx QlNX1Uw+E4Erd2Lu6/PJNqURShzEzjym9I1simnJ+hjARsSAOhsm8zwP+8gBCTKOgP1A h7vQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694206327; x=1694811127; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :sender:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=VDbHt9ZjxbWg0OnwI6q2vK5uaDqhBGdPDoTGYH2kvhs=; b=YBB+ftbv5+8GXMXQFOEAezN/trg4Z04WWSnXIPqU/5NhXwylWwuenjmu79qD7XUsj1 qCDt8rlHqoYp4jXLAgE6GxEf1Pm3geCy7mZH3thID2tRWc88RoPNi+bWX7af0mqbu62/ ZMr4yRgdbAGClKDAzVtUQj5OE+vCuDhca8JsMXOcqbSncrtRQjGGvi3lddmIiDzImj+f wYJYMjwSCGSs14wE50uVQy5meJRVMXM40dFv8VSqLGWU3cQ610FOEKckPTyUs4alHjIE zYNI6uiQC0thxi+lx0YRvE9RjJTTTyC5OAbKQO+IBbXf5oQXSKk3oBCkOoORvFMDtrsI OciQ== X-Gm-Message-State: AOJu0Yz6nLkInZ+uiXpYiOF67UXOVJvGPb5rMYtI5XkBMr7WpQuMxCb2 C/4UQ7b65BQTp6UOaH2ye/k= X-Google-Smtp-Source: AGHT+IE3hfS2KA7c98Htv9+NTlTCBJzkYRjkcPnycGW13NYWt9fF/InbMS0k6WcKTOaP72T/K0kq5w== X-Received: by 2002:a05:6a21:60f:b0:153:beda:863 with SMTP id ll15-20020a056a21060f00b00153beda0863mr3012237pzb.45.1694206327330; Fri, 08 Sep 2023 13:52:07 -0700 (PDT) Received: from ?IPV6:2600:1700:e321:62f0:329c:23ff:fee3:9d7c? ([2600:1700:e321:62f0:329c:23ff:fee3:9d7c]) by smtp.gmail.com with ESMTPSA id m11-20020aa78a0b000000b0068a53ac9d46sm1667577pfa.100.2023.09.08.13.52.05 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 08 Sep 2023 13:52:06 -0700 (PDT) Sender: Guenter Roeck Message-ID: Date: Fri, 8 Sep 2023 13:52:04 -0700 Precedence: bulk X-Mailing-List: loongarch@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH] lockdep: Fix static memory detection even more Content-Language: en-US To: Helge Deller , Huacai Chen , loongarch@lists.linux.dev Cc: linux-kernel@vger.kernel.org, "Rafael J. Wysocki" , Borislav Petkov , Peter Zijlstra References: <78473084-d4d5-685f-9778-4bbe8878a43e@roeck-us.net> <081537c0-0d74-0242-451a-e6bd6f71cdd9@roeck-us.net> From: Guenter Roeck In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 9/7/23 00:40, Helge Deller wrote: > * Guenter Roeck : >> On 9/6/23 00:18, Helge Deller wrote: >>> * Guenter Roeck : >>>> On 9/3/23 14:11, Helge Deller wrote: >>>>> * Guenter Roeck : >>>>>> Hi, >>>>>> >>>>>> On Sat, Aug 12, 2023 at 05:48:52PM +0200, Helge Deller wrote: >>>>>>> On the parisc architecture, lockdep reports for all static objects which >>>>>>> are in the __initdata section (e.g. "setup_done" in devtmpfs, >>>>>>> "kthreadd_done" in init/main.c) this warning: >>>>>>> >>>>>>> INFO: trying to register non-static key. >>>>>>> >>>>>>> The warning itself is wrong, because those objects are in the __initdata >>>>>>> section, but the section itself is on parisc outside of range from >>>>>>> _stext to _end, which is why the static_obj() functions returns a wrong >>>>>>> answer. >>>>>>> >>>>>>> While fixing this issue, I noticed that the whole existing check can >>>>>>> be simplified a lot. >>>>>>> Instead of checking against the _stext and _end symbols (which include >>>>>>> code areas too) just check for the .data and .bss segments (since we check a >>>>>>> data object). This can be done with the existing is_kernel_core_data() >>>>>>> macro. >>>>>>> >>>>>>> In addition objects in the __initdata section can be checked with >>>>>>> init_section_contains(). >>>>>>> >>>>>>> This partly reverts and simplifies commit bac59d18c701 ("x86/setup: Fix static >>>>>>> memory detection"). >>>>>>> >>>>>>> Tested on x86-64 and parisc. >>>>>>> >>>>>>> Signed-off-by: Helge Deller >>>>>>> Fixes: bac59d18c701 ("x86/setup: Fix static memory detection") >>>>>> >>>>>> On loongarch, this patch results in the following backtrace. >>>>>> >>>>>> EFI stub: Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path >>>>>> EFI stub: Exiting boot services >>>>>> [ 0.000000] INFO: trying to register non-static key. >>>>>> [ 0.000000] The code is fine but needs lockdep annotation, or maybe >>>>>> [ 0.000000] you didn't initialize this object before use? >>>>>> [ 0.000000] turning off the locking correctness validator. >>>>>> [ 0.000000] ... >>>>>> [ 0.000000] Call Trace: >>>>>> [ 0.000000] [<9000000000223d84>] show_stack+0x5c/0x180 >>>>>> [ 0.000000] [<900000000153e0b4>] dump_stack_lvl+0x88/0xd0 >>>>>> [ 0.000000] [<90000000002bc548>] register_lock_class+0x768/0x770 >>>>>> [ 0.000000] [<90000000002bc710>] __lock_acquire+0xb0/0x2a18 >>>>>> [ 0.000000] [<90000000002bba1c>] lock_acquire+0x11c/0x328 >>>>>> [ 0.000000] [<9000000000b34a60>] __debug_object_init+0x60/0x244 >>>>>> [ 0.000000] [<9000000000337f94>] init_cgroup_housekeeping+0xe8/0x144 >>>>>> [ 0.000000] [<900000000033e364>] init_cgroup_root+0x38/0xa0 >>>>>> [ 0.000000] [<90000000017801ac>] cgroup_init_early+0x44/0x16c >>>>>> [ 0.000000] [<9000000001770758>] start_kernel+0x50/0x624 >>>>>> [ 0.000000] [<90000000015410b4>] kernel_entry+0xb4/0xc4 >>>>>> >>>>>> Reverting it fixes the problem. Bisect log attached. >>>>>> >>>>>> This is also seen in v6.5.y and v6.4.y since the patch has been applied >>>>>> to those branches. >>>>> >>>>> Does this happens with CONFIG_SMP=n ? >>>>> If so, I think the untested patch below might fix the issue. >>>>> >>>> >>>> No, this is loongarch:defconfig with various debug options enabled. >>>> That has CONFIG_SMP=y. >>> >>> Could you apply below patch and verify with the contents of the >>> System.map file where the lock is located ? >>> >>> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c >>> index e85b5ad3e206..db0a301f9740 100644 >>> --- a/kernel/locking/lockdep.c >>> +++ b/kernel/locking/lockdep.c >>> @@ -969,7 +969,7 @@ static bool assign_lock_key(struct lockdep_map *lock) >>> else { >>> /* Debug-check: all keys must be persistent! */ >>> debug_locks_off(); >>> - pr_err("INFO: trying to register non-static key.\n"); >>> + pr_err("INFO: trying to register non-static key at %08lx.\n", addr); >>> pr_err("The code is fine but needs lockdep annotation, or maybe\n"); >>> pr_err("you didn't initialize this object before use?\n"); >>> pr_err("turning off the locking correctness validator.\n"); >> >> 90000000015602d0 D __la_abs_end >> ... >> 90000000016815c0 d fill_pool_map.3 <--- lock pointer >> ... >> 9000000001770000 T __init_begin > > The problem is, that loongarch's vmlinux.lds.S file puts data into > areas which are not marked "data" or "code". > > can you try below patch? > The relevant part is here the move of ".data.rel : { *(.data.rel*) }" > into the data section. > The other parts are suggestions. > The patch below fixes the problem for me. Guenter > Helge > > > diff --git a/arch/loongarch/kernel/vmlinux.lds.S b/arch/loongarch/kernel/vmlinux.lds.S > index b1686afcf876..bb2ec86f37a8 100644 > --- a/arch/loongarch/kernel/vmlinux.lds.S > +++ b/arch/loongarch/kernel/vmlinux.lds.S > @@ -53,33 +53,6 @@ SECTIONS > . = ALIGN(PECOFF_SEGMENT_ALIGN); > _etext = .; > > - /* > - * struct alt_inst entries. From the header (alternative.h): > - * "Alternative instructions for different CPU types or capabilities" > - * Think locking instructions on spinlocks. > - */ > - . = ALIGN(4); > - .altinstructions : AT(ADDR(.altinstructions) - LOAD_OFFSET) { > - __alt_instructions = .; > - *(.altinstructions) > - __alt_instructions_end = .; > - } > - > -#ifdef CONFIG_RELOCATABLE > - . = ALIGN(8); > - .la_abs : AT(ADDR(.la_abs) - LOAD_OFFSET) { > - __la_abs_begin = .; > - *(.la_abs) > - __la_abs_end = .; > - } > -#endif > - > - .got : ALIGN(16) { *(.got) } > - .plt : ALIGN(16) { *(.plt) } > - .got.plt : ALIGN(16) { *(.got.plt) } > - > - .data.rel : { *(.data.rel*) } > - > . = ALIGN(PECOFF_SEGMENT_ALIGN); > __init_begin = .; > __inittext_begin = .; > @@ -94,6 +67,18 @@ SECTIONS > > __initdata_begin = .; > > + /* > + * struct alt_inst entries. From the header (alternative.h): > + * "Alternative instructions for different CPU types or capabilities" > + * Think locking instructions on spinlocks. > + */ > + . = ALIGN(4); > + .altinstructions : AT(ADDR(.altinstructions) - LOAD_OFFSET) { > + __alt_instructions = .; > + *(.altinstructions) > + __alt_instructions_end = .; > + } > + > INIT_DATA_SECTION(16) > .exit.data : { > EXIT_DATA > @@ -113,6 +98,11 @@ SECTIONS > > _sdata = .; > RO_DATA(4096) > + > + .got : ALIGN(16) { *(.got) } > + .plt : ALIGN(16) { *(.plt) } > + .got.plt : ALIGN(16) { *(.got.plt) } > + > RW_DATA(1 << CONFIG_L1_CACHE_SHIFT, PAGE_SIZE, THREAD_SIZE) > > .rela.dyn : ALIGN(8) { > @@ -121,6 +111,17 @@ SECTIONS > __rela_dyn_end = .; > } > > + .data.rel : { *(.data.rel*) } > + > +#ifdef CONFIG_RELOCATABLE > + . = ALIGN(8); > + .la_abs : AT(ADDR(.la_abs) - LOAD_OFFSET) { > + __la_abs_begin = .; > + *(.la_abs) > + __la_abs_end = .; > + } > +#endif > + > .sdata : { > *(.sdata) > } >