imp (Warner Losh)
User

Projects

User Details

User Since
Jun 2 2014, 4:20 PM (156 w, 18 h)

Recent Activity

Sat, May 27

imp added inline comments to D10931: boot1 generate-fat: generate all templates at once.
Sat, May 27, 6:06 PM
imp added inline comments to D10931: boot1 generate-fat: generate all templates at once.
Sat, May 27, 5:59 PM
imp added a comment to D10948: Allow to both the forth and raw command line in the loader.

I'm not entirely sure that switching interpreters is a good idea. I rather think it's a terrible idea, but am open to seeing how it progresses before rendering final judgement.

Sat, May 27, 4:40 PM
imp added inline comments to D10948: Allow to both the forth and raw command line in the loader.
Sat, May 27, 4:37 PM
imp added a comment to D10642: Fix serial line terminal size..

This is starting to sound like a terminal emulation type vs term= miss match perhaps? My term size was quiet correct, that is what a standard Putty serial terminal size comes up as.

Sat, May 27, 4:33 PM
imp added inline comments to D10931: boot1 generate-fat: generate all templates at once.
Sat, May 27, 12:03 AM

Fri, May 26

imp added a comment to D10489: Add EXAMPLES section to uname(1).

OK, let's see if I understood you correctly this time Warner. make(1) is indeed apparently setting MACHINE and MACHINE_ARCH globally.

Fri, May 26, 11:35 PM · manpages

Thu, May 25

imp added a comment to D10642: Fix serial line terminal size..

Isnt this caused by a missing entry in /etc/ttys for the serial console? This should be coming from there, not from .dot files.

Thu, May 25, 1:51 AM

Mon, May 22

imp accepted D10764: sys/: Remove glimpse make target added in r181432.

I don't know if there's folks still using glimpse from before... But it's been long enough that I think it can go.

Mon, May 22, 2:57 PM

Sat, May 20

imp added a comment to D10787: Expose the ILP32/LP64 programming environments based on __ILP32__/__LP64__ instead of by architecture..
In D10787#224240, @ngie wrote:
In D10787#223634, @imp wrote:
In D10787#223484, @ngie wrote:

Only arm64 exposes __ILP32__ and __LP64__ (depending on whether or not it's 32-bit or 64-bit), per a quick grep of contrib/gcc and contrib/llvm .

All the architectures do. We use it elsewhere in the tree and it works. Your grep has lead you astray.

contrib/gcc/config/i386/freebsd64.h: builtin_define ("LP64"); \
contrib/gcc/config/rs6000/freebsd.h: builtin_define ("LP64"); \
contrib/gcc/config/sparc/freebsd.h: builtin_define ("LP64"); \

are just three examples.

I meant to say "only arm64 exposes it, in addition to the other architectures currently handled".

What I'm wondering about thought is why aren't arm and mips in this list at all? And why does powerpc assume 32 bit?

Beats me why arm/mips were never considered, but there is an issue potentially with how __ILP32__, __LP32__, and __LP64__ are defined on those architectures.

This review specifically addresses the hardcoded have_<FOO> for LP64 and ILP32 that was incorrect (maybe not when this was first written, but wasn't correct after additional architectures were added).

Sat, May 20, 12:12 AM

Thu, May 18

imp added a comment to D10787: Expose the ILP32/LP64 programming environments based on __ILP32__/__LP64__ instead of by architecture..
In D10787#223484, @ngie wrote:

Only arm64 exposes __ILP32__ and __LP64__ (depending on whether or not it's 32-bit or 64-bit), per a quick grep of contrib/gcc and contrib/llvm .

Thu, May 18, 3:12 PM
imp requested changes to D10787: Expose the ILP32/LP64 programming environments based on __ILP32__/__LP64__ instead of by architecture..

Actually I think I take that back... mips and arm aren't listed at all. Are they affected?

Thu, May 18, 6:57 AM
imp accepted D10787: Expose the ILP32/LP64 programming environments based on __ILP32__/__LP64__ instead of by architecture..
Thu, May 18, 6:55 AM

Wed, May 17

imp accepted D10537: Make sure <arch-gcc> come with consistent content.

Looks good to my eye as well...

Wed, May 17, 6:03 PM
imp accepted D10642: Fix serial line terminal size..

Looks good to me.

Wed, May 17, 12:08 AM

Tue, May 16

imp added a comment to D10489: Add EXAMPLES section to uname(1).

Strictly speaking, you are correct, but man pages often have references elsewhere, and having it here wouldn't be terrible.

Understood, fair enough. Would .Xr build 7 reference suffice (it already establishes a connection between variables and uname -[mp] output there), or you'd rather duplicate this information explicitly here as well?

Tue, May 16, 3:53 PM · manpages

Mon, May 15

imp added a comment to D10489: Add EXAMPLES section to uname(1).

Guys, I don't entirely understand how TARGET/MACHINE and/or TARGET/MACHINE_ARCH come into play here: this patch is about uname(1) only; these variables are not referenced anywhere in its sources, and are partially (?) documented in build(7). Can you elaborate a bit?

Mon, May 15, 5:27 PM · manpages
imp accepted D10741: e1000api: misleading-indentation.
Mon, May 15, 2:00 PM

Thu, May 11

imp added a comment to D10645: Avoid use of contiguous memory allocations in busdma.

Comments need to be updated to reflect the change in the last bit of the if.

Thu, May 11, 2:27 PM

Tue, May 9

imp accepted D10654: Fix build(7) example..

OK. I like this. However, I have a suggestion (inspired by Ravi's comment) for improvement that should be done, but you may consider it beyond the scope of this change.

Tue, May 9, 4:46 PM
imp added a comment to D10642: Fix serial line terminal size..

Putting it there would be better.

Tue, May 9, 1:37 PM

Mon, May 8

imp added inline comments to D10617: Add initial support for the floating point implementation register..
Mon, May 8, 5:08 PM
imp added a comment to D10642: Fix serial line terminal size..

Pi, not Pie.

Mon, May 8, 1:21 PM

Fri, May 5

imp added inline comments to D10573: arch.7: add table of initial FreeBSD version to support each arch.
Fri, May 5, 9:27 PM

Thu, May 4

imp accepted D10596: More human voice for arch(7)..

Thanks for the tweaks! I'm happy now.

Thu, May 4, 4:48 PM
imp added inline comments to D10596: More human voice for arch(7)..
Thu, May 4, 4:47 PM
imp added inline comments to D10596: More human voice for arch(7)..
Thu, May 4, 4:26 PM

Tue, May 2

imp added inline comments to D10573: arch.7: add table of initial FreeBSD version to support each arch.
Tue, May 2, 5:10 PM
imp added inline comments to D10573: arch.7: add table of initial FreeBSD version to support each arch.
Tue, May 2, 4:55 PM
imp added inline comments to D10573: arch.7: add table of initial FreeBSD version to support each arch.
Tue, May 2, 4:52 PM
imp accepted D10568: Document time_t size..

Looks good to me.

Tue, May 2, 2:21 PM

Apr 27 2017

imp accepted D10520: Various fixes for PCI _OSC handling so HotPlug works again..

This looks good to my eye. But I wrote the original, so take that with a grain of salt.

Apr 27 2017, 5:37 AM

Apr 26 2017

imp added a comment to D10466: Fix NVMe's use of XPT_GDEV_TYPE.

Remove it. If we need it again, it can be added back.

Apr 26 2017, 8:30 PM

Apr 25 2017

imp added a comment to D10489: Add EXAMPLES section to uname(1).

If "platform" (-m) and "processor architecture" (-p) are the same as the TARGET and TARGET_ARCH, then it would be great to include that too.

Apr 25 2017, 1:13 PM · manpages

Apr 22 2017

imp added a comment to D10457: Add ability to label md(4) devices.

Love this idea, but the padding issue needs to be resolved.

Apr 22 2017, 3:55 AM

Apr 19 2017

imp accepted D10425: net: Add new 10G and 25G media types to if_media.h.

You could ifdef the things for which version supports it. But that interacts poorly with the ports system, so I'd go for it on bumping the FreeBSD_version. It's cheap enough.

Apr 19 2017, 7:34 PM · network

Apr 16 2017

imp accepted D10399: Disable in-tree GDB by default on x86, mips, and powerpc..

I think this is a good chance. I advocated doing this over a year ago now I think :)

Apr 16 2017, 11:33 PM

Apr 11 2017

imp accepted D9674: Handle ENXIO bufs..

I'm OK with this version.

Apr 11 2017, 8:07 PM
imp added a comment to D7997: makeman: don't copy $FreeBSD$ tags from source files into output.

Yea, I agree that the use here doesn't get us much.

Apr 11 2017, 4:57 PM

Apr 10 2017

imp accepted D7997: makeman: don't copy $FreeBSD$ tags from source files into output.

I'm OK not having the IDs used to generate things... They don't provide that much info in this case, and the ID for the file will get us traceability.

Apr 10 2017, 10:12 PM

Apr 7 2017

imp accepted D10247: Add CAM passthrough for NVMe.

Looks good to me.

Apr 7 2017, 9:24 PM
imp accepted D10302: loader: r316585 did miss sparc/ofw and userboot update.

looks good to me.

Apr 7 2017, 4:27 PM
imp accepted D10307: Define 'lr' as x30 on aarch64.

lgtm

Apr 7 2017, 3:38 PM
imp added inline comments to D10305: Do not use b.cs instruction to jump to cerror.
Apr 7 2017, 3:34 PM
imp accepted D10303: Remove the last vestiges of FDC_DEBUG & FD_DEBUG.

Looks to be a remnant of the pre-phk floppy driver rewrite.

Apr 7 2017, 1:28 AM
imp added a comment to D10302: loader: r316585 did miss sparc/ofw and userboot update.
In D10302#213231, @imp wrote:

This suggests that getting the size is wrong and using it the way you do is flawed.

For the pool, yes, we really should get the partition size eventually - assuming we want to get the correct location for the last 2 labels.

The backup GPT is defined to be where the primary GPT says it is, which is not necessarily the last sector(s).

yea, but in this case it is not really even about GPT, as the old sparcs only do support vtoc and GPT support was added for T4 and up. Anyhow, as you do mention GPT, for practical purposes using the GPT backup label location is enough, as it is the last thing *after* the data, and so that will give us good size limit in any case, even if the physical device is larger, it has no data after the backup label.

Apr 7 2017, 12:16 AM

Apr 6 2017

imp added a comment to D10302: loader: r316585 did miss sparc/ofw and userboot update.

This suggests that getting the size is wrong and using it the way you do is flawed.

Apr 6 2017, 11:23 PM
imp accepted D10180: loader: part.c cstyle cleanup.
Apr 6 2017, 3:09 PM

Apr 5 2017

imp accepted D10085: Explicitly pass ABI flags for MIPS..

Looks good to me. Some minor tweaks will be needed for some other issues, but those are beyond the scope of these changes so this is good to go.

Apr 5 2017, 7:37 PM

Apr 4 2017

imp added inline comments to D10247: Add CAM passthrough for NVMe.
Apr 4 2017, 4:07 PM

Apr 3 2017

imp accepted D10085: Explicitly pass ABI flags for MIPS..

OK. I'm cool with this... As we've noted there's issues here, but other things need to be fixed before we can address the criticisms. And that isn't important enough to hold up this patch for.

Apr 3 2017, 11:36 PM
imp added a comment to D10253: Add capability for newsyslog to write RFC5424 compliant rotation message..

Separating the commits is good code hygiene. While the project doesn't always do it, it's an important skill for our contributors to have (knowing how to do the splits as well as using tools that make that easy). Otherwise, we tend to wind up with long, meandering commits that are hard to bisect when there's the inevitable problem.

Apr 3 2017, 8:19 PM
imp added inline comments to D10247: Add CAM passthrough for NVMe.
Apr 3 2017, 2:59 PM
imp accepted D10239: Create a machine/metadata.h for RISC-V.

Why does sys/crypto/crypto.c include this at all?

Apr 3 2017, 1:01 AM

Mar 29 2017

imp committed rS316171: xfsread inlined uses more space, so remove the inline tag. This.
xfsread inlined uses more space, so remove the inline tag. This
Mar 29 2017, 6:36 PM

Mar 28 2017

imp committed rS316100: Remove -fno-guess-branch-probability and -fno-unit-at-a-time..
Remove -fno-guess-branch-probability and -fno-unit-at-a-time.
Mar 28 2017, 6:09 PM
imp committed rS316079: Simply retire the sedification of the boot2.s file. It's been obsolete.
Simply retire the sedification of the boot2.s file. It's been obsolete
Mar 28 2017, 7:58 AM

Mar 27 2017

imp committed rS316064: Fix build with path names with 'align' or 'nop' in them..
Fix build with path names with 'align' or 'nop' in them.
Mar 27 2017, 10:53 PM

Mar 24 2017

imp committed rS315918: Add 'device iic' to bring in userland I2C driver..
Add 'device iic' to bring in userland I2C driver.
Mar 24 2017, 10:33 PM
imp committed rS315901: Use a more stream-lined version of fix_value..
Use a more stream-lined version of fix_value.
Mar 24 2017, 1:46 PM

Mar 23 2017

imp closed D6286: Implement quote escaping. String values may now contain " if you it is preceded by \. by committing rS315773: Implement quote escaping. String values may now contain " if you.
Mar 23 2017, 2:37 AM
imp committed rS315773: Implement quote escaping. String values may now contain " if you.
Implement quote escaping. String values may now contain " if you
Mar 23 2017, 2:37 AM
imp committed rS315770: Define StrCmp in a funky was to be bug-compatible with EDK2 code..
Define StrCmp in a funky was to be bug-compatible with EDK2 code.
Mar 23 2017, 2:31 AM
imp committed rS315771: Fix a coverity-discovered NULL pointer dereference..
Fix a coverity-discovered NULL pointer dereference.
Mar 23 2017, 2:31 AM

Mar 22 2017

imp committed rS315740: Simplify the code a little..
Simplify the code a little.
Mar 22 2017, 8:52 PM
imp committed rS315735: Implement moving SD..
Implement moving SD.
Mar 22 2017, 7:19 PM
imp committed rS315733: Impelemnt ttys onifexists in init..
Impelemnt ttys onifexists in init.
Mar 22 2017, 7:00 PM
imp closed D10037: Impelemnt ttys onifexists in init. by committing rS315733: Impelemnt ttys onifexists in init..
Mar 22 2017, 7:00 PM
imp added inline comments to D10085: Explicitly pass ABI flags for MIPS..
Mar 22 2017, 6:59 PM
imp accepted D10096: usr.sbin.makefs.makefs_cd9660_tests.F_flag fails on a kernel missing 'device cd'.

Same thing 4 times... Maybe a routine would be better?

Mar 22 2017, 4:44 PM
imp accepted D10085: Explicitly pass ABI flags for MIPS..

I generally like this change. We tried very hard to require the system compiler to do the right thing for two reasons back in the day. First, we wanted to make sure a naked cc could just build things so ports would work better. Second, there were several tricky, hard-coded at the time, bits in different builds that got the wrong ABI tags and so wouldn't link (hack.so?) that were a pita to track down... If the latter has been solved, the former does remain. However, I think it's better that we're explicit since it enables a wider range of compiler to build the base system, and the problem of using those compilers with ports is orthogonal. We shouldn't require it any longer since it is standing in the way of making things better. I regret that we decided to do what we did back in the day since it has held back the external-toolchain work on mips a bit.

Mar 22 2017, 3:59 AM

Mar 20 2017

imp accepted D9970: libcam: NULL out freed `ccb.cdm.matches` and `ccb.cdm.patterns` pointers to avoid double frees.

Thanks for making the suggested changes...

Mar 20 2017, 3:39 PM

Mar 17 2017

imp added a comment to D10041: Stop building assym.o in modules.

Once upon a time this was needed to create the assym.h file that encoded all the struct sizes and offsets the .S files used. No clue why it is here though....

Mar 17 2017, 8:22 PM

Mar 16 2017

imp added inline comments to D10037: Impelemnt ttys onifexists in init..
Mar 16 2017, 8:50 PM
imp added inline comments to D10037: Impelemnt ttys onifexists in init..
Mar 16 2017, 8:40 PM
imp created D10037: Impelemnt ttys onifexists in init..
Mar 16 2017, 8:09 PM

Mar 14 2017

imp accepted D9996: Fix a memory leak in ccdconfig.
Mar 14 2017, 4:56 PM
imp accepted D9577: Add the possibility to use another DTC.

Looks like all of Bryan's feedback has been answered, and that answers the questions I had.

Mar 14 2017, 4:01 PM · ARM

Mar 13 2017

imp added inline comments to D9970: libcam: NULL out freed `ccb.cdm.matches` and `ccb.cdm.patterns` pointers to avoid double frees.
Mar 13 2017, 5:13 PM
imp added a comment to D9970: libcam: NULL out freed `ccb.cdm.matches` and `ccb.cdm.patterns` pointers to avoid double frees.
In D9970#206301, @ngie wrote:
In D9970#206077, @imp wrote:

I like setting persistent things to NULL after free, but setting local variables to NULL after free on the 'exit' path of the function is actually harmful: Either the compiler will generate code that wastes cycles in what could be a critical path, or it will optimize it away and maybe issue a warning.... Neither outcome is good, and there's no benefit to doing this.

I forgot to reply to this portion of the comment: could you please provide an example of this? I know that running free/munmap on a large collection of memory pages can eat up a lot of time unnecessarily on exit, but I'm not quite sure I understand the negative effects you're mentioning above.

Mar 13 2017, 4:46 PM
imp accepted D9968: lib/libcam/cam_cdbparse.3: fix manpage warnings.

Generally looks good, modulo some of the text. But it's not a text cleanup, per se, so I'm good with this going in.

Mar 13 2017, 4:32 PM

Mar 12 2017

imp committed rS315175: Convert gnu to using SRCTOP.
Convert gnu to using SRCTOP
Mar 12 2017, 6:59 PM
imp committed rS315174: Make rescue use SRCTOP.
Make rescue use SRCTOP
Mar 12 2017, 6:59 PM
imp committed rS315172: Convert include over to SRCTOP.
Convert include over to SRCTOP
Mar 12 2017, 6:59 PM
imp committed rS315173: Fix two CURDIR references in comments that should be SRCTOP.
Fix two CURDIR references in comments that should be SRCTOP
Mar 12 2017, 6:59 PM
imp committed rS315171: Move /etc/ to SRCTOP.
Move /etc/ to SRCTOP
Mar 12 2017, 6:59 PM
imp committed rS315170: Adopt SRCTOP in usr.bin.
Adopt SRCTOP in usr.bin
Mar 12 2017, 6:59 PM
imp closed D9932: Prefer ${SRCTOP}/foo over ${.CURDIR}/../../foo and ${SRCTOP}/usr.bin/foo over ${.CURDIR}/../foo for paths in Makefiles. by committing rS315170: Adopt SRCTOP in usr.bin.
Mar 12 2017, 6:59 PM
imp committed rS315166: Export the actual LID state via sysctl. This allows more complex.
Export the actual LID state via sysctl. This allows more complex
Mar 12 2017, 6:00 PM
imp added a comment to D9970: libcam: NULL out freed `ccb.cdm.matches` and `ccb.cdm.patterns` pointers to avoid double frees.

I like setting persistent things to NULL after free, but setting local variables to NULL after free on the 'exit' path of the function is actually harmful: Either the compiler will generate code that wastes cycles in what could be a critical path, or it will optimize it away and maybe issue a warning.... Neither outcome is good, and there's no benefit to doing this.

Mar 12 2017, 4:37 PM
imp added a comment to D9969: lib/libcam/cam.3: note that cam_freeccb(3) with ccb == NULL is a no-op.

Forget what I said about anything being missing, other than my ability to read for comprehension.

Mar 12 2017, 4:32 PM
imp accepted D9969: lib/libcam/cam.3: note that cam_freeccb(3) with ccb == NULL is a no-op.

Note that the text doesn't match the description. The description is missing the word 'NULL' I think.

Mar 12 2017, 4:31 PM

Mar 10 2017

imp added a comment to D9915: Use of reallocarray(3) in userland..

I'm agnostic to the change.
I get why they are doing malloc(a * b) -> reallocarray(NULL, a, b), but wonder if it would be better to have a wider API that's mallocarray(a,b) if you are looking for syntactic goodness along with your overflow protection....

Mar 10 2017, 5:06 PM
imp updated the diff for D9932: Prefer ${SRCTOP}/foo over ${.CURDIR}/../../foo and ${SRCTOP}/usr.bin/foo over ${.CURDIR}/../foo for paths in Makefiles..
  • Move /etc/ to SRCTOP
  • Convert include over to SRCTOP
  • Fix two CURDIR references in comments that should be SRCTOP
  • Make rescue use SRCTOP
  • Fold with usr.bin
  • Convert gnu/lib to using SRCTOP
  • gnu/usr.bin SRCTOP
Mar 10 2017, 6:25 AM
imp added a comment to D9932: Prefer ${SRCTOP}/foo over ${.CURDIR}/../../foo and ${SRCTOP}/usr.bin/foo over ${.CURDIR}/../foo for paths in Makefiles..

I think I overstate it with "everywhere: I think "in selected places" might be a better term. We use the gmake equivalent $(abspath $(somepath)) a lot and it makes things a lot more readable. As for the speed of it, whatever gmake does doesnt seem to take so long as I've never noticed a slowdown.

Mar 10 2017, 4:35 AM
imp added a comment to D9932: Prefer ${SRCTOP}/foo over ${.CURDIR}/../../foo and ${SRCTOP}/usr.bin/foo over ${.CURDIR}/../foo for paths in Makefiles..

just my $0.02

Mar 10 2017, 3:04 AM

Mar 9 2017

imp updated the summary of D9932: Prefer ${SRCTOP}/foo over ${.CURDIR}/../../foo and ${SRCTOP}/usr.bin/foo over ${.CURDIR}/../foo for paths in Makefiles..
Mar 9 2017, 6:06 PM
imp created D9932: Prefer ${SRCTOP}/foo over ${.CURDIR}/../../foo and ${SRCTOP}/usr.bin/foo over ${.CURDIR}/../foo for paths in Makefiles..
Mar 9 2017, 6:05 PM
imp accepted D9927: Fix typo in nvme(4).
Mar 9 2017, 4:56 AM
imp accepted D9907: Port i.MX6 to PLATFORM_SMP.

This looks good to my eye. One nit.

Mar 9 2017, 4:55 AM