- User Since
- Jul 9 2015, 9:56 PM (294 w, 12 h)
Tue, Feb 23
Thu, Feb 18
Still accepted from earlier
Outside of ossl_chacha20.c, everything looks fine.
Hm, it is little endian, but I'm not confident about the two sentences prior.
Wed, Feb 17
Tue, Feb 16
Mon, Feb 15
The analysis makes sense and the fix looks fine to me. You could allocate two fds and check socketpair against both (instead of just low) in the final test, but I don't think there's much marginal value in that. (Frankly, I'm not a fan of unix' must-assign-lowest-fd behavior in general, but we have to abide by it.)
Sat, Feb 13
Fri, Feb 12
In the commit summary, is this a typo?
Mon, Feb 8
No objection in principle, but the diff is basically unreviewable as-is for me; it's like 90% autoconf generated shell.
Tue, Feb 2
Mon, Feb 1
Looks great to me!
Fri, Jan 29
Thu, Jan 28
That's true, but it's possible for callers to depend on the instantaneous result being correct because some external synchronization guarantees that the result is stable. For instance, some lock might serialize calls to getblk() for a particular vnode, so holding that lock ensures that incore() returns a stable result. I'm worried about false negatives caused by reassignbuf() in cases like this.
I think the idea here is that CRC32 is part of SSE4.2, which is part of the base feature set of amd64. If you're hitting SIGILL in QEMU in amd64 mode, that suggests QEMU's amd64 emulation is somewhat invalid. However, it's certainly optional on at least some early models of i386.
Wed, Jan 27
I think incore was already always racy? The race scenarios may have changed slightly.
Jan 26 2021
Jan 23 2021
Jan 22 2021
I think maybe we should revert the change, if it is truly unfounded and was only observed on 12. If this wasn't ever a problem on CURRENT, the commit message rationale is wrong and misleading. Reverting is the canonical way to correct that kind of thing.
Wait, kldxref should be ordering these correctly since rS348309. Why isn't it?
Jan 17 2021
Jan 14 2021
Sure, seems fine to me.
Jan 9 2021
Jan 8 2021
I don’t know anything about cscope but it looks fine. Ideally files that would only be found in the root (eg tinderbox crap) are “/“ prefixed to reduce the search cost in subdirectories. Orthogonal to this change.
Jan 4 2021
Jan 3 2021
Ok, so the bug was that we never took any lock at all? But we're also moving a block of code surrounding the new vn_lock(); operation, which isn't related to our not-locking-the-vp-bug. It looks like we want to reject EINVAL flags before we start locking, which is fine. I would just like to see a better commit message than "fix a [non-specific] bug." Something like "add missing lock in fuse advlock. also, reject invalid parameters sooner (style)."
The other nice thing about this routine (ok, conditional on debug.vfs_badlock_vnode=1, but it is the default) — or VNASSERT(), VPASS() — is that they'll print out details of the vp. MPASS can't do that.
Dec 30 2020
The make-bits and other integration look fine. I didn't have time to verify the meat of the implementation (and probably won't).
Mostly LGTM, thanks!
Dec 27 2020
Dec 25 2020
Dec 24 2020
Dec 22 2020
Dec 17 2020
Re: hyphenated metadata attribute names — I'd just keep our annotations the same as they are now. If we were starting from scratch as a git project, sure, we should adopt the common git scheme. But the annotations are not special in git (just part of the commit message), and any FreeBSD tooling has to understand our previous annotations anyway.
Dec 16 2020
Can we lift the condition out and just have two switches? Not sure if that’s any better but it seems sort of ugly as is.
Dec 12 2020
Dec 11 2020
I like the approach, thanks!
Thanks for adding the new hardware. Seems like a reasonable approach to me.
Dec 10 2020
Is kernel.KERNCONF actually a convention? I tend to use INSTKERNNAME and 'test.KERNCONF'.
Dec 9 2020
I'm not sure I understand the solution, but totally believe there's a bug in this area.
Dec 8 2020
Dec 6 2020
Dec 5 2020
Thanks for the quick look, folks!