Re: buildkernel is broken
On Thu, Jul 07, 2022 at 07:59:29PM +0200, Kristof Provost wrote: > On 7 Jul 2022, at 19:00, Steve Kargl wrote: > > > The fix in > > 37f604b49d4a seems rather questionable especially given > > that there is no comment about why the macro is expanded > > to a zero-trip loop. > > > I’m not sure how I could have been much more clear than this: > > VNET_FOREACH() is a LIST_FOREACH if VIMAGE is set, but empty if it's > not. This means that users of the macro couldn't use 'continue' or > 'break' as one would expect of a loop. Comments belong in the code. /* Kludge to prevent non-vimage kernels from choking to death. */ #define VNET_FOREACH(arg) for (int _vn = 0; _vn == 0; _vn++) -- Steve
Re: buildkernel is broken
On 7 Jul 2022, at 19:00, Steve Kargl wrote: On Thu, Jul 07, 2022 at 10:37:40AM -0600, Warner Losh wrote: On Thu, Jul 7, 2022 at 10:37 AM Steve Kargl < s...@troutmask.apl.washington.edu> wrote: Thanks, but root[216] git cherry-pick -n 37f604b49d4a fatal: bad revision '37f604b49d4a' root[217] pwd /usr/src git fetch maybe? A cursory google search suggests that 'git fetch' works on repositories not single files. I did look at the diff associated with 37f604b49d4a. I am surprised that the commit that broke buildkernel for me was allowed to be committed. It was posted for review in https://reviews.freebsd.org/D35716 I’ll also point out that this commit works just fine in nearly all of our kernel configs, because there are very few (only one powerpc config, as far as I can tell) that do not have VIMAGE. Arguably we should have a non-VIMAGE kernel config around (probably for amd64) so it’s more likely we spot these issues prior to commit. Arbitrary non-default kernel configs are more likely to see issues like this one. I don’t think that can be avoided. The fix in 37f604b49d4a seems rather questionable especially given that there is no comment about why the macro is expanded to a zero-trip loop. I’m not sure how I could have been much more clear than this: VNET_FOREACH() is a LIST_FOREACH if VIMAGE is set, but empty if it's not. This means that users of the macro couldn't use 'continue' or 'break' as one would expect of a loop. I welcome suggestions on how to improve my future commit messages. To rephrase it a bit: VNET_FOREACH() used to be very misleading, in that it was only a loop with options VIMAGE, and empty (so any code within would be its own block, and be executed exactly once, for the only vnet that exists without VIMAGE). That’s fine, unless you want to ‘continue’ or ‘break’ the loop. That worked with VIMAGE (so the issue in the dummynet fix was not seen) but not without it. Kristof
Re: buildkernel is broken
On Thu, Jul 7, 2022 at 11:21 AM Kyle Evans wrote: > On Thu, Jul 7, 2022 at 10:01 AM Steve Kargl > wrote: > > > > On Thu, Jul 07, 2022 at 10:37:40AM -0600, Warner Losh wrote: > > > On Thu, Jul 7, 2022 at 10:37 AM Steve Kargl < > > > s...@troutmask.apl.washington.edu> wrote: > > > > > > > Thanks, but > > > > > > > > root[216] git cherry-pick -n 37f604b49d4a > > > > fatal: bad revision '37f604b49d4a' > > > > root[217] pwd > > > > /usr/src > > > > > > > > > git fetch maybe? > > > > > > > A cursory google search suggests that 'git fetch' > > works on repositories not single files. > > > > Right, the idea is that you `git fetch origin main` (or whatever your > remote is called rather than 'origin') then you can cherry-pick the > revision. fetch will grab the revision without merging it into the > current branch. > Indeed. a git fetch command will only update the remote/origin/upstream tags and such as well as fetching new revisions there. It won't affect any local branches as all until you explicitly merge them with 'git merge' or implicitly with a 'git pull'. Warner > > I did look at the diff associated with 37f604b49d4a. > > I am surprised that the commit that broke buildkernel > > for me was allowed to be committed. The fix in > > 37f604b49d4a seems rather questionable especially given > > that there is no comment about why the macro is expanded > > to a zero-trip loop. > > > > Thanks for the help. I'll just do a 'git pull' > > and start over with a buildworld. > > > > -- > > Steve > > >
Re: buildkernel is broken
On Thu, Jul 7, 2022 at 10:01 AM Steve Kargl wrote: > > On Thu, Jul 07, 2022 at 10:37:40AM -0600, Warner Losh wrote: > > On Thu, Jul 7, 2022 at 10:37 AM Steve Kargl < > > s...@troutmask.apl.washington.edu> wrote: > > > > > Thanks, but > > > > > > root[216] git cherry-pick -n 37f604b49d4a > > > fatal: bad revision '37f604b49d4a' > > > root[217] pwd > > > /usr/src > > > > > > git fetch maybe? > > > > A cursory google search suggests that 'git fetch' > works on repositories not single files. > Right, the idea is that you `git fetch origin main` (or whatever your remote is called rather than 'origin') then you can cherry-pick the revision. fetch will grab the revision without merging it into the current branch. > I did look at the diff associated with 37f604b49d4a. > I am surprised that the commit that broke buildkernel > for me was allowed to be committed. The fix in > 37f604b49d4a seems rather questionable especially given > that there is no comment about why the macro is expanded > to a zero-trip loop. > > Thanks for the help. I'll just do a 'git pull' > and start over with a buildworld. > > -- > Steve >
Re: buildkernel is broken
On Thu, Jul 07, 2022 at 10:37:40AM -0600, Warner Losh wrote: > On Thu, Jul 7, 2022 at 10:37 AM Steve Kargl < > s...@troutmask.apl.washington.edu> wrote: > > > Thanks, but > > > > root[216] git cherry-pick -n 37f604b49d4a > > fatal: bad revision '37f604b49d4a' > > root[217] pwd > > /usr/src > > > git fetch maybe? > A cursory google search suggests that 'git fetch' works on repositories not single files. I did look at the diff associated with 37f604b49d4a. I am surprised that the commit that broke buildkernel for me was allowed to be committed. The fix in 37f604b49d4a seems rather questionable especially given that there is no comment about why the macro is expanded to a zero-trip loop. Thanks for the help. I'll just do a 'git pull' and start over with a buildworld. -- Steve
Re: buildkernel is broken
On Thu, Jul 7, 2022 at 10:37 AM Steve Kargl < s...@troutmask.apl.washington.edu> wrote: > Thanks, but > > root[216] git cherry-pick -n 37f604b49d4a > fatal: bad revision '37f604b49d4a' > root[217] pwd > /usr/src git fetch maybe? Warner > -- > steve > > On Thu, Jul 07, 2022 at 11:24:47AM -0400, Ryan Stone wrote: > > You could "git cherry-pick -n 37f604b49d4a; git restore --unstaged > > sys/net/vnet.h" to apply the fix to your local tree without committing > > it or leaving it staged for commit. > > > > On Thu, Jul 7, 2022 at 10:50 AM Steve Kargl > > wrote: > > > > > > On Thu, Jul 07, 2022 at 10:38:43AM -0400, Ryan Stone wrote: > > > > Okay, update your tree and it should be fixed then. > > > > > > Is it possible to pull just that fix? I spent part of > > > yesterday building world, and contrary to popular belief, > > > not all hardware contain a 32-core uber-fast ryzen cpu. > > > > > > Can people please test their simple changes prior to > > > committing? > > > > > > -- > > > Steve > > -- > Steve > >
Re: buildkernel is broken
Thanks, but root[216] git cherry-pick -n 37f604b49d4a fatal: bad revision '37f604b49d4a' root[217] pwd /usr/src -- steve On Thu, Jul 07, 2022 at 11:24:47AM -0400, Ryan Stone wrote: > You could "git cherry-pick -n 37f604b49d4a; git restore --unstaged > sys/net/vnet.h" to apply the fix to your local tree without committing > it or leaving it staged for commit. > > On Thu, Jul 7, 2022 at 10:50 AM Steve Kargl > wrote: > > > > On Thu, Jul 07, 2022 at 10:38:43AM -0400, Ryan Stone wrote: > > > Okay, update your tree and it should be fixed then. > > > > Is it possible to pull just that fix? I spent part of > > yesterday building world, and contrary to popular belief, > > not all hardware contain a 32-core uber-fast ryzen cpu. > > > > Can people please test their simple changes prior to > > committing? > > > > -- > > Steve -- Steve
Re: buildkernel is broken
Or you could cherry-pick the fix. When you update git pull --rebase will automatically drop it when you rebase past that spot. Or if you have a branch, you can do the same to the branch and when you rebase past the fix, it will automatically drop. Warner On Thu, Jul 7, 2022 at 9:25 AM Ryan Stone wrote: > You could "git cherry-pick -n 37f604b49d4a; git restore --unstaged > sys/net/vnet.h" to apply the fix to your local tree without committing > it or leaving it staged for commit. > > On Thu, Jul 7, 2022 at 10:50 AM Steve Kargl > wrote: > > > > On Thu, Jul 07, 2022 at 10:38:43AM -0400, Ryan Stone wrote: > > > Okay, update your tree and it should be fixed then. > > > > Is it possible to pull just that fix? I spent part of > > yesterday building world, and contrary to popular belief, > > not all hardware contain a 32-core uber-fast ryzen cpu. > > > > Can people please test their simple changes prior to > > committing? > > > > -- > > Steve > >
Re: buildkernel is broken
You could "git cherry-pick -n 37f604b49d4a; git restore --unstaged sys/net/vnet.h" to apply the fix to your local tree without committing it or leaving it staged for commit. On Thu, Jul 7, 2022 at 10:50 AM Steve Kargl wrote: > > On Thu, Jul 07, 2022 at 10:38:43AM -0400, Ryan Stone wrote: > > Okay, update your tree and it should be fixed then. > > Is it possible to pull just that fix? I spent part of > yesterday building world, and contrary to popular belief, > not all hardware contain a 32-core uber-fast ryzen cpu. > > Can people please test their simple changes prior to > committing? > > -- > Steve
Re: buildkernel is broken
On Thu, Jul 07, 2022 at 10:38:43AM -0400, Ryan Stone wrote: > Okay, update your tree and it should be fixed then. Is it possible to pull just that fix? I spent part of yesterday building world, and contrary to popular belief, not all hardware contain a 32-core uber-fast ryzen cpu. Can people please test their simple changes prior to committing? -- Steve
Re: buildkernel is broken
Okay, update your tree and it should be fixed then.
Re: buildkernel is broken
Yes, I do. Deleting the line allows to code to compile. -- steve On Thu, Jul 07, 2022 at 08:24:47AM -0400, Ryan Stone wrote: > Do you have VNET disabled in your kernel config? I believe that this > was fixed by 37f604b49d4a. > > On Thu, Jul 7, 2022 at 1:07 AM Steve Kargl > wrote: > > > > -std=iso9899:1999 -Werror /usr/src/sys/netinet/tcp_input.c > > --- modules-all --- > > /usr/src/sys/netpfil/ipfw/ip_dn_io.c:674:4: error: 'continue' statement not > > in loop statement > > continue; > > ^ > > 1 error generated. > > *** [ip_dn_io.o] Error code 1 > > > > make[4]: stopped in /usr/src/sys/modules/dummynet > > 1 error > > *** [modules-all] Error code 6 > > > > make[2]: stopped in /usr/obj/usr/src/amd64.amd64/sys/SPEW > > 1 error > > > > make[2]: stopped in /usr/obj/usr/src/amd64.amd64/sys/SPEW > > 5.75 real20.45 user 2.30 sys > > > > make[1]: stopped in /usr/src > > > > > > Please fix. > > > > -- > > Steve > > -- Steve
Re: buildkernel is broken
Do you have VNET disabled in your kernel config? I believe that this was fixed by 37f604b49d4a. On Thu, Jul 7, 2022 at 1:07 AM Steve Kargl wrote: > > -std=iso9899:1999 -Werror /usr/src/sys/netinet/tcp_input.c > --- modules-all --- > /usr/src/sys/netpfil/ipfw/ip_dn_io.c:674:4: error: 'continue' statement not > in loop statement > continue; > ^ > 1 error generated. > *** [ip_dn_io.o] Error code 1 > > make[4]: stopped in /usr/src/sys/modules/dummynet > 1 error > *** [modules-all] Error code 6 > > make[2]: stopped in /usr/obj/usr/src/amd64.amd64/sys/SPEW > 1 error > > make[2]: stopped in /usr/obj/usr/src/amd64.amd64/sys/SPEW > 5.75 real20.45 user 2.30 sys > > make[1]: stopped in /usr/src > > > Please fix. > > -- > Steve >
Re: buildkernel is broken
On Sun, Jul 07, 2013 at 11:51:01PM +0200, Claude Buisson wrote: C On 07/07/2013 22:05, Gleb Smirnoff wrote: C On Fri, Jul 05, 2013 at 08:32:06AM -0700, Steve Kargl wrote: C S On Fri, Jul 05, 2013 at 05:03:38PM +0400, Gleb Smirnoff wrote: C S On Tue, Jul 02, 2013 at 08:45:16PM -0700, Steve Kargl wrote: C S S On Tue, Jul 02, 2013 at 10:51:57PM -0230, Jonathan Anderson wrote: C S S On Tuesday, 2 July 2013 at 22:16, Steve Kargl wrote: C S S It seems that C S S C S S # Enable FreeBSD7 compatibility syscalls C S S options COMPAT_FREEBSD7 C S S C S S is required in a kernel config file. If it is mandatory to C S S have this option on FreeBSD 10, it may be appropriate to C S S expand the comment to C S S C S S C S S # Enable FreeBSD7 compatibility syscalls C S S # This option is MANDATORY. Do not remove. C S S options COMPAT_FREEBSD7 C S S C S S So... a non-optional option? C S S C S S Yes, it appears to be that way. C S C S Not really. It is required only if you also got COMPAT_43, the C S pre-FreeBSD compat option. C S C S C S So, it's essentially non-optional as the comment above COMPAT_43 C S is C S C S # C S # Implement system calls compatible with 4.3BSD and older versions of C S # FreeBSD. You probably do NOT want to remove this as much current code C S # still relies on the 4.3 emulation. C S C S Is do NOT want to remove too strong of a statement? C C Where did you find this statement? I remember it, but haven't seen C it for long time. Can't find it in modern GENERIC. C C C Try and read sys/conf/NOTES I think, the statement is outdated today. -- Totus tuus, Glebius. ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: buildkernel is broken
On Fri, Jul 05, 2013 at 08:32:06AM -0700, Steve Kargl wrote: S On Fri, Jul 05, 2013 at 05:03:38PM +0400, Gleb Smirnoff wrote: S On Tue, Jul 02, 2013 at 08:45:16PM -0700, Steve Kargl wrote: S S On Tue, Jul 02, 2013 at 10:51:57PM -0230, Jonathan Anderson wrote: S S On Tuesday, 2 July 2013 at 22:16, Steve Kargl wrote: S S It seems that S S S S # Enable FreeBSD7 compatibility syscalls S S options COMPAT_FREEBSD7 S S S S is required in a kernel config file. If it is mandatory to S S have this option on FreeBSD 10, it may be appropriate to S S expand the comment to S S S S S S # Enable FreeBSD7 compatibility syscalls S S # This option is MANDATORY. Do not remove. S S options COMPAT_FREEBSD7 S S S S So... a non-optional option? S S S S Yes, it appears to be that way. S S Not really. It is required only if you also got COMPAT_43, the S pre-FreeBSD compat option. S S S So, it's essentially non-optional as the comment above COMPAT_43 S is S S # S # Implement system calls compatible with 4.3BSD and older versions of S # FreeBSD. You probably do NOT want to remove this as much current code S # still relies on the 4.3 emulation. S S Is do NOT want to remove too strong of a statement? Where did you find this statement? I remember it, but haven't seen it for long time. Can't find it in modern GENERIC. -- Totus tuus, Glebius. ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: buildkernel is broken
On 07/07/2013 22:05, Gleb Smirnoff wrote: On Fri, Jul 05, 2013 at 08:32:06AM -0700, Steve Kargl wrote: S On Fri, Jul 05, 2013 at 05:03:38PM +0400, Gleb Smirnoff wrote: S On Tue, Jul 02, 2013 at 08:45:16PM -0700, Steve Kargl wrote: S S On Tue, Jul 02, 2013 at 10:51:57PM -0230, Jonathan Anderson wrote: S S On Tuesday, 2 July 2013 at 22:16, Steve Kargl wrote: S S It seems that S S S S # Enable FreeBSD7 compatibility syscalls S S options COMPAT_FREEBSD7 S S S S is required in a kernel config file. If it is mandatory to S S have this option on FreeBSD 10, it may be appropriate to S S expand the comment to S S S S S S # Enable FreeBSD7 compatibility syscalls S S # This option is MANDATORY. Do not remove. S S options COMPAT_FREEBSD7 S S S S So... a non-optional option? S S S S Yes, it appears to be that way. S S Not really. It is required only if you also got COMPAT_43, the S pre-FreeBSD compat option. S S S So, it's essentially non-optional as the comment above COMPAT_43 S is S S # S # Implement system calls compatible with 4.3BSD and older versions of S # FreeBSD. You probably do NOT want to remove this as much current code S # still relies on the 4.3 emulation. S S Is do NOT want to remove too strong of a statement? Where did you find this statement? I remember it, but haven't seen it for long time. Can't find it in modern GENERIC. Try and read sys/conf/NOTES CBu ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: buildkernel is broken
On Tue, Jul 02, 2013 at 08:45:16PM -0700, Steve Kargl wrote: S On Tue, Jul 02, 2013 at 10:51:57PM -0230, Jonathan Anderson wrote: S On Tuesday, 2 July 2013 at 22:16, Steve Kargl wrote: S It seems that S S # Enable FreeBSD7 compatibility syscalls S options COMPAT_FREEBSD7 S S is required in a kernel config file. If it is mandatory to S have this option on FreeBSD 10, it may be appropriate to S expand the comment to S S S # Enable FreeBSD7 compatibility syscalls S # This option is MANDATORY. Do not remove. S options COMPAT_FREEBSD7 S S So... a non-optional option? S S Yes, it appears to be that way. Not really. It is required only if you also got COMPAT_43, the pre-FreeBSD compat option. -- Totus tuus, Glebius. ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: buildkernel is broken
On Fri, Jul 05, 2013 at 05:03:38PM +0400, Gleb Smirnoff wrote: On Tue, Jul 02, 2013 at 08:45:16PM -0700, Steve Kargl wrote: S On Tue, Jul 02, 2013 at 10:51:57PM -0230, Jonathan Anderson wrote: S On Tuesday, 2 July 2013 at 22:16, Steve Kargl wrote: S It seems that S S # Enable FreeBSD7 compatibility syscalls S options COMPAT_FREEBSD7 S S is required in a kernel config file. If it is mandatory to S have this option on FreeBSD 10, it may be appropriate to S expand the comment to S S S # Enable FreeBSD7 compatibility syscalls S # This option is MANDATORY. Do not remove. S options COMPAT_FREEBSD7 S S So... a non-optional option? S S Yes, it appears to be that way. Not really. It is required only if you also got COMPAT_43, the pre-FreeBSD compat option. So, it's essentially non-optional as the comment above COMPAT_43 is # # Implement system calls compatible with 4.3BSD and older versions of # FreeBSD. You probably do NOT want to remove this as much current code # still relies on the 4.3 emulation. Is do NOT want to remove too strong of a statement? -- Steve ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: buildkernel is broken
On Tue, Jul 02, 2013 at 05:35:35PM -0700, Steve Kargl wrote: /usr/src/sys/kern/sysv_shm.c: At top level: /usr/src/sys/kern/sysv_shm.c:1044: error: 'freebsd7_shmctl' undeclared here (not in a function) *** Error code 1 It seems that # Enable FreeBSD7 compatibility syscalls options COMPAT_FREEBSD7 is required in a kernel config file. If it is mandatory to have this option on FreeBSD 10, it may be appropriate to expand the comment to # Enable FreeBSD7 compatibility syscalls # This option is MANDATORY. Do not remove. options COMPAT_FREEBSD7 -- Steve ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: buildkernel is broken
On Tuesday, 2 July 2013 at 22:16, Steve Kargl wrote: It seems that # Enable FreeBSD7 compatibility syscalls options COMPAT_FREEBSD7 is required in a kernel config file. If it is mandatory to have this option on FreeBSD 10, it may be appropriate to expand the comment to # Enable FreeBSD7 compatibility syscalls # This option is MANDATORY. Do not remove. options COMPAT_FREEBSD7 So... a non-optional option? Jon -- Jonathan Anderson jonat...@freebsd.org ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: buildkernel is broken
On Tue, Jul 02, 2013 at 10:51:57PM -0230, Jonathan Anderson wrote: On Tuesday, 2 July 2013 at 22:16, Steve Kargl wrote: It seems that # Enable FreeBSD7 compatibility syscalls options COMPAT_FREEBSD7 is required in a kernel config file. If it is mandatory to have this option on FreeBSD 10, it may be appropriate to expand the comment to # Enable FreeBSD7 compatibility syscalls # This option is MANDATORY. Do not remove. options COMPAT_FREEBSD7 So... a non-optional option? Yes, it appears to be that way. -- Steve ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
