From nobody Thu Jan 18 21:47:37 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4TGGZH1Xg7z5758w for ; Thu, 18 Jan 2024 21:47:51 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4TGGZG6mwNz4nsy for ; Thu, 18 Jan 2024 21:47:50 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-wm1-x335.google.com with SMTP id 5b1f17b1804b1-40e7e2e04f0so1244285e9.1 for ; Thu, 18 Jan 2024 13:47:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20230601.gappssmtp.com; s=20230601; t=1705614469; x=1706219269; darn=freebsd.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=U7W8V3V7WOTk/Nr7yZARoa8uVuLUDDaKvk/kPdvKXC4=; b=hMtU6GnaL/tzIVY0/1p7aPKskdQfNgyw76aqOL0jpneBe//YWnJSgGD0y679pIRH8c RmkUaVZM01yvSi2LnpQ9D/KibTuqSlTKxMy+JjaDxfRVXWZYqEnKgF5haoSAQi1jr3nE 5uVywu4/8IgnE5eOq9e0Qpe9KAzjaYuYn33Ft7Lj9Lz4lAsk/adD7qHxYtM3axhwrN05 RQwMr1QjXB3FbjUop6FYFb3LVygmYfAhJhlbIYgB9FDmIlLSwAhm8r9bwzjDrvzG9u/d QGRnCgiu+xwt70ml8KW2jZwGlUl/Nr87ZclZg/aySJKWlRHNvD/ydkD7YDM/eeunFUhm Idgg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705614469; x=1706219269; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=U7W8V3V7WOTk/Nr7yZARoa8uVuLUDDaKvk/kPdvKXC4=; b=cBMnk3UvHxxntr+Su38RX76U9WA+izAM2Za8g34vuof0Oi2JQ9KbPhQdVqH037MoM2 pkTFpghN9orI7edzxfqOhKXrpfepRCzZUnw4JacmIeTMlyYpCkOTrteaqVlD1G1x0jrr jFCofchTCaVSvbuGbrDK0hWX2QkcYPH6Gi6Y898utLLicJio7YESUakDQv4yAX1Z+2fY W9E4QwpDeCJhvg6LY7aDE2gNRI+Ay3uHG9x/4vyR0VE8FtnZVKs6yAu7171nyNOG1bj6 L1WBV/6gjmjFqY5fpKBBtq5vtg4LZj7MekVHVA8tUVDPLvL/ZgkcRfH3+W7mZmKlCwL5 WgkA== X-Gm-Message-State: AOJu0YxB0LaNS9oTAUeD9wD57H1JsqMOlEbNTkeKiDyJUE7z4IHpi6YX wVQVFFtRwpknFINS4Yog3YGQTe0JZ/rPVvXMHK/zdWz4n2gMPFA5aVZEccC4KvrQXB9AyRb51En D9/B947If2AFWhFYaF66/e8tbgNby3lOWdvqjjOBkeFH7+beV64TEqQ== X-Google-Smtp-Source: AGHT+IHEO7heJxqYjMmxnncPKR3EAXvFCUDpDcYG9NnRGNpgkXOJZwzLBpkNkj8qo7ne7wl95w2kYRFhRQFMGs6Uiao= X-Received: by 2002:a05:600c:19cb:b0:40e:50e9:9b0c with SMTP id u11-20020a05600c19cb00b0040e50e99b0cmr854768wmq.181.1705614469179; Thu, 18 Jan 2024 13:47:49 -0800 (PST) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 References: <202302251737.31PHb2R8072300@gitrepo.freebsd.org> In-Reply-To: From: Warner Losh Date: Thu, 18 Jan 2024 14:47:37 -0700 Message-ID: Subject: Re: git: 773c13c686e4 - main - kldxref: skip .pkgsave files To: John Baldwin Cc: Warner Losh , src-committers , dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: multipart/alternative; boundary="0000000000008f3cd6060f3f506b" X-Rspamd-Queue-Id: 4TGGZG6mwNz4nsy X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US] --0000000000008f3cd6060f3f506b Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Doh! This is what I have hanging around diff --git a/usr.sbin/kldxref/kldxref.c b/usr.sbin/kldxref/kldxref.c index 1694f069564b..25bfab7129e1 100644 --- a/usr.sbin/kldxref/kldxref.c +++ b/usr.sbin/kldxref/kldxref.c @@ -842,10 +842,10 @@ main(int argc, char *argv[]) continue; /* * Skip files that generate errors like .debug, .symbol and .pkgsave - * by generally skipping all files with 2 dots. + * by generally skipping all files not ending with ".ko". */ - dot =3D strchr(p->fts_name, '.'); - if (dot && strchr(dot + 1, '.') !=3D NULL) + dot =3D strrchr(p->fts_name, '.'); + if (dot =3D=3D NULL || strcmp(dot, ".ko") !=3D 0) continue; read_kld(p->fts_path, p->fts_name); } See anything obviously wrong with it before I make it into a phab? Sorry for the delay... Warner On Thu, Jan 18, 2024 at 2:35=E2=80=AFPM John Baldwin wrot= e: > On 12/6/23 5:35 PM, Warner Losh wrote: > > On Wed, Dec 6, 2023, 2:53 PM John Baldwin wrote: > > > >> On 12/6/23 1:41 PM, Warner Losh wrote: > >>> Hey John, > >>> > >>> On Wed, Dec 6, 2023 at 2:13=E2=80=AFPM John Baldwin = wrote: > >>> > >>>> On 12/6/23 1:02 PM, Warner Losh wrote: > >>>>> On Wed, Dec 6, 2023, 1:04 PM John Baldwin wrote: > >>>>> > >>>>>> On 2/25/23 9:37 AM, Warner Losh wrote: > >>>>>>> The branch main has been updated by imp: > >>>>>>> > >>>>>>> URL: > >>>>>> > >>>> > >> > https://cgit.FreeBSD.org/src/commit/?id=3D773c13c686e4b6ae9dbbc150b342b82= c3f47d73a > >>>>>>> > >>>>>>> commit 773c13c686e4b6ae9dbbc150b342b82c3f47d73a > >>>>>>> Author: Mina Gali=C4=87 > >>>>>>> AuthorDate: 2023-02-25 17:31:58 +0000 > >>>>>>> Commit: Warner Losh > >>>>>>> CommitDate: 2023-02-25 17:35:43 +0000 > >>>>>>> > >>>>>>> kldxref: skip .pkgsave files > >>>>>>> > >>>>>>> This should help people transitioning from traditional > setups > >> to > >>>>>> pkgbase > >>>>>>> experience a lot less friction. > >>>>>>> > >>>>>>> We do this by skipping all files containing two dots. > >>>>>>> > >>>>>>> Reviewed by: imp > >>>>>>> Pull Request: > https://github.com/freebsd/freebsd-src/pull/661 > >>>>>>> Differential Revision: https://reviews.freebsd.org/D27959 > >>>>>> > >>>>>> This restriction is too broad and omits all of the modern wifi > >> firmware > >>>>>> klds from linker.hints, e.g. > >>>>>> > >>>>>> /boot/kernel/iwlwifi-3160-17.ucode.ko > >>>>>> /boot/kernel/iwlwifi-3168-29.ucode.ko > >>>>>> /boot/kernel/iwlwifi-7260-17.ucode.ko > >>>>>> /boot/kernel/iwlwifi-7265-17.ucode.ko > >>>>>> /boot/kernel/iwlwifi-7265D-29.ucode.ko > >>>>>> /boot/kernel/iwlwifi-8000C-36.ucode.ko > >>>>>> /boot/kernel/iwlwifi-8265-36.ucode.ko > >>>>>> /boot/kernel/iwlwifi-9000-pu-b0-jf-b0-46.ucode.ko > >>>>>> /boot/kernel/iwlwifi-9260-th-b0-jf-b0-46.ucode.ko > >>>>>> /boot/kernel/iwlwifi-Qu-b0-hr-b0-77.ucode.ko > >>>>>> /boot/kernel/iwlwifi-Qu-b0-jf-b0-77.ucode.ko > >>>>>> /boot/kernel/iwlwifi-Qu-c0-hr-b0-77.ucode.ko > >>>>>> /boot/kernel/iwlwifi-Qu-c0-jf-b0-77.ucode.ko > >>>>>> /boot/kernel/iwlwifi-QuZ-a0-hr-b0-77.ucode.ko > >>>>>> /boot/kernel/iwlwifi-QuZ-a0-jf-b0-77.ucode.ko > >>>>>> /boot/kernel/iwlwifi-cc-a0-77.ucode.ko > >>>>>> /boot/kernel/iwlwifi-so-a0-gf-a0-83.ucode.ko > >>>>>> /boot/kernel/iwlwifi-so-a0-gf-a0.pnvm.ko > >>>>>> /boot/kernel/iwlwifi-so-a0-gf4-a0-83.ucode.ko > >>>>>> /boot/kernel/iwlwifi-so-a0-gf4-a0.pnvm.ko > >>>>>> /boot/kernel/iwlwifi-so-a0-hr-b0-81.ucode.ko > >>>>>> /boot/kernel/iwlwifi-so-a0-jf-b0-77.ucode.ko > >>>>>> /boot/kernel/iwlwifi-ty-a0-gf-a0-83.ucode.ko > >>>>>> /boot/kernel/iwlwifi-ty-a0-gf-a0.pnvm.ko > >>>>>> /boot/kernel/rtw8723d_fw.bin.ko > >>>>>> /boot/kernel/rtw8821c_fw.bin.ko > >>>>>> /boot/kernel/rtw8822b_fw.bin.ko > >>>>>> /boot/kernel/rtw8822c_fw.bin.ko > >>>>>> /boot/kernel/rtw8822c_wow_fw.bin.ko > >>>>>> > >>>>>> all match this pattern and are skipped. > >>>>>> > >>>>>> I'm busy rewriting a bunch of kldxref to be a cross tool using > libelf, > >>>>>> but I think here you want to probably revert this and just add > pkgsave > >>>>>> to the list of "known bad" suffixes. > >>>>>> > >>>>> > >>>>> Sure. Any reason to not just require .ko? Or do we have to index th= e > >>>> kernel > >>>>> too? > >>>> > >>>> We do index the kernel as well, yes. However, we could probably get > by > >>>> with "kernel" and ends in ".ko" as a valid set of files. This would > >> also > >>>> avoid bogusly warning about linker.hints not being a valid ELF file = on > >>>> re-runs if you use -v. > >>>> > >>> > >>> Yea, that sounds good. I'll code it up and add you to the review. > >>> > >>> But why does it matter for these? Firmware is usually loaded by > filename > >>> and need not be elf... or are these wrapped in elf sections... > >>> > >>> I haven't noticed it breaking my linuxkpi wifi driver that have > >> autoloaded > >>> firmware... > >> > >> Hmm, afaik firmwares are loaded by "module name" where a firmware .ko > >> contains > >> one or more of the firmware modules. We happen today to generally onl= y > >> store one module in a single .ko (and with the same name), and in that > case > >> kern_linker.c may fallback to just trying to load "foo".ko if it doesn= 't > >> find > >> an entry in linker.hints, but if that is why it is working that is > >> certainly > >> by happy accident. > >> > >> I only found this by comparing klxref output btw on a stale i386 VM > between > >> the native kldxref in the VM (before this change) and my cross-arch > version > >> of kldxref. > >> > > > > Ok. That all makes sense. I'll update my working tree tomorrow with the > > revert and the replacement. Since it "works" today, I'll push the rever= t > > and the fix at the same time unless the review takes too long. > > Ping, do you still have this fix in your pending tree? I noticed it is > still there when doing MFC's of the libelf kldxref changes today. > > -- > John Baldwin > > --0000000000008f3cd6060f3f506b Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Doh! This is what I have hanging around

diff --git a/usr.sbin/kldxref/kldxref.c b/usr.sbin/kldxref/kldxref.c
in= dex 1694f069564b..25bfab7129e1 100644
--- a/usr.sbin/kldxref/kldxref.c+++ b/usr.sbin/kldxref/kldxref.c
@@ -842,10 +842,10 @@ main(int argc, = char *argv[])
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 continue;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 /*
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0* Skip files that generate errors like .debug, .symbol and= .pkgsave
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* by generally skip= ping all files with 2 dots.
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* by gen= erally skipping all files not ending with ".ko".
=C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
- =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 dot =3D strchr(p->fts_name, '.');
- =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (dot && strchr(dot + 1, '.')= !=3D NULL)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 dot =3D strrchr(p->fts_name= , '.');
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (dot =3D=3D NULL || str= cmp(dot, ".ko") !=3D 0)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 continue;
=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 read_kld(p->fts_path, p->ft= s_name);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }

See = anything obviously wrong with it before I make it into a phab?
Sorry for the delay...

Warner

On= Thu, Jan 18, 2024 at 2:35=E2=80=AFPM John Baldwin <jhb@freebsd.org> wrote:
On 12/6/23 5:35 PM, Warner Losh wrote:
> On Wed, Dec 6, 2023, 2:53 PM John Baldwin <jhb@freebsd.org> wrote:
>
>> On 12/6/23 1:41 PM, Warner Losh wrote:
>>> Hey John,
>>>
>>> On Wed, Dec 6, 2023 at 2:13=E2=80=AFPM John Baldwin <jhb@freebsd.org> wrot= e:
>>>
>>>> On 12/6/23 1:02 PM, Warner Losh wrote:
>>>>> On Wed, Dec 6, 2023, 1:04 PM John Baldwin <jhb@freebsd.org> wrote= :
>>>>>
>>>>>> On 2/25/23 9:37 AM, Warner Losh wrote:
>>>>>>> The branch main has been updated by imp:
>>>>>>>
>>>>>>> URL:
>>>>>>
>>>>
>> https://c= git.FreeBSD.org/src/commit/?id=3D773c13c686e4b6ae9dbbc150b342b82c3f47d73a
>>>>>>>
>>>>>>> commit 773c13c686e4b6ae9dbbc150b342b82c3f47d73= a
>>>>>>> Author:=C2=A0 =C2=A0 =C2=A0Mina Gali=C4=87 <= ;freebsd@igalic.co>
>>>>>>> AuthorDate: 2023-02-25 17:31:58 +0000
>>>>>>> Commit:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp= @FreeBSD.org>
>>>>>>> CommitDate: 2023-02-25 17:35:43 +0000
>>>>>>>
>>>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0kldxref: skip= .pkgsave files
>>>>>>>
>>>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0This should h= elp people transitioning from traditional setups
>> to
>>>>>> pkgbase
>>>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0experience a = lot less friction.
>>>>>>>
>>>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0We do this by= skipping all files containing two dots.
>>>>>>>
>>>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Reviewed by: = imp
>>>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Pull Request:= https://github.com/freebsd/freebsd-src/pull/661<= br> >>>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Differential = Revision: https://reviews.freebsd.org/D27959
>>>>>>
>>>>>> This restriction is too broad and omits all of the= modern wifi
>> firmware
>>>>>> klds from linker.hints, e.g.
>>>>>>
>>>>>> /boot/kernel/iwlwifi-3160-17.ucode.ko
>>>>>> /boot/kernel/iwlwifi-3168-29.ucode.ko
>>>>>> /boot/kernel/iwlwifi-7260-17.ucode.ko
>>>>>> /boot/kernel/iwlwifi-7265-17.ucode.ko
>>>>>> /boot/kernel/iwlwifi-7265D-29.ucode.ko
>>>>>> /boot/kernel/iwlwifi-8000C-36.ucode.ko
>>>>>> /boot/kernel/iwlwifi-8265-36.ucode.ko
>>>>>> /boot/kernel/iwlwifi-9000-pu-b0-jf-b0-46.ucode.ko<= br> >>>>>> /boot/kernel/iwlwifi-9260-th-b0-jf-b0-46.ucode.ko<= br> >>>>>> /boot/kernel/iwlwifi-Qu-b0-hr-b0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-Qu-b0-jf-b0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-Qu-c0-hr-b0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-Qu-c0-jf-b0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-QuZ-a0-hr-b0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-QuZ-a0-jf-b0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-cc-a0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-so-a0-gf-a0-83.ucode.ko
>>>>>> /boot/kernel/iwlwifi-so-a0-gf-a0.pnvm.ko
>>>>>> /boot/kernel/iwlwifi-so-a0-gf4-a0-83.ucode.ko
>>>>>> /boot/kernel/iwlwifi-so-a0-gf4-a0.pnvm.ko
>>>>>> /boot/kernel/iwlwifi-so-a0-hr-b0-81.ucode.ko
>>>>>> /boot/kernel/iwlwifi-so-a0-jf-b0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-ty-a0-gf-a0-83.ucode.ko
>>>>>> /boot/kernel/iwlwifi-ty-a0-gf-a0.pnvm.ko
>>>>>> /boot/kernel/rtw8723d_fw.bin.ko
>>>>>> /boot/kernel/rtw8821c_fw.bin.ko
>>>>>> /boot/kernel/rtw8822b_fw.bin.ko
>>>>>> /boot/kernel/rtw8822c_fw.bin.ko
>>>>>> /boot/kernel/rtw8822c_wow_fw.bin.ko
>>>>>>
>>>>>> all match this pattern and are skipped.
>>>>>>
>>>>>> I'm busy rewriting a bunch of kldxref to be a = cross tool using libelf,
>>>>>> but I think here you want to probably revert this = and just add pkgsave
>>>>>> to the list of "known bad" suffixes.
>>>>>>
>>>>>
>>>>> Sure. Any reason to not just require .ko? Or do we hav= e to index the
>>>> kernel
>>>>> too?
>>>>
>>>> We do index the kernel as well, yes.=C2=A0 However, we cou= ld probably get by
>>>> with "kernel" and ends in ".ko" as a v= alid set of files.=C2=A0 This would
>> also
>>>> avoid bogusly warning about linker.hints not being a valid= ELF file on
>>>> re-runs if you use -v.
>>>>
>>>
>>> Yea, that sounds good. I'll code it up and add you to the = review.
>>>
>>> But why does it matter for these? Firmware is usually loaded b= y filename
>>> and need not be elf... or are these wrapped in elf sections...=
>>>
>>> I haven't noticed it breaking my linuxkpi wifi driver that= have
>> autoloaded
>>> firmware...
>>
>> Hmm, afaik firmwares are loaded by "module name" where a= firmware .ko
>> contains
>> one or more of the firmware modules.=C2=A0 We happen today to gene= rally only
>> store one module in a single .ko (and with the same name), and in = that case
>> kern_linker.c may fallback to just trying to load "foo".= ko if it doesn't
>> find
>> an entry in linker.hints, but if that is why it is working that is=
>> certainly
>> by happy accident.
>>
>> I only found this by comparing klxref output btw on a stale i386 V= M between
>> the native kldxref in the VM (before this change) and my cross-arc= h version
>> of kldxref.
>>
>
> Ok. That all makes sense. I'll update my working tree tomorrow wit= h the
> revert and the replacement. Since it "works" today, I'll= push the revert
> and the fix at the same time unless the review takes too long.

Ping, do you still have this fix in your pending tree?=C2=A0 I noticed it i= s
still there when doing MFC's of the libelf kldxref changes today.

--
John Baldwin

--0000000000008f3cd6060f3f506b--