User Details
- User Since
- Jun 2 2014, 4:20 PM (529 w, 4 d)
Yesterday
I'm unsure about this
Nothing except zfs_fallthrough is needed.
It does undo the ACCESS_ONCE part of it, but doesn't undo the proper uintptr_t casting part. Oh, but I see the const cast is there for READ...
I can omit that bit too. OpenZFS really just reads READ_ONCE and WRITE_ONCE anyway. It
suffers from defining this stuff in too many different places :(.
I'd prefer a bunch of small, self-contained commits. I'd tag them all with this one review. Phabricator isn't ideal with that, but it's the best we have.
That way, we can MFC them piecemeal if need be, and people trying to 'merge all FOO driver changes' or whatever will see the right thing.
If you're worried, push a branch to github and I can take a look.
I see the warning in my builds, but it keeps going...
I have a stale review comment somewhere I can't find now. But it said something like "I like this change because we do way too much inlining in the linuxkpi because that causes problems with our goal to have a stable ABI on a stable branch"
A question about volatile away from the review.
Having both atomics and volatile isn't the usual pattern and the CW has usually been that adding volatile won't fix code like this, so either remove volatile, or comment about what it accomplishes likely is appropriate even if it's a brief reference....
Do we care about Arm 8.0? Or will __ARM_FEATURE_ATOMICS not be defined there?
There's other inconsistent stuff here, but this looks save since the data is protected by a lock.
more reduction
Thu, Jul 25
Yea, I hadn't planned on including the second paragraph...
I kinda like moving to __deprecated... but we can do that separately after the final form is agreed to.
This has landed, though in too many commits to document here.
This landed differently
Wed, Jul 24
Any idea how to test this?
Will something else killing, eg, camcontrol which has issued the command before the I/O is complete be a good test?
I guess I'm nervous for reasons that maybe are completly irrational and looking for some way to test this...
Can we document which processes/tools do that?
"delete nop that's now no longer called" :)
It wouldn't hurt to note in the commit message that this brings us into compliance with posix.1-2024 on this point. Many people may not know why the austin group matters
I love it when a plan comes together so nicely. Agree 100% that this should be deterministic
Apart from the one comment I left, I like it. Here's more context on some of these drivers than maybe you wanted...
With kib's changes...
I was worried this would always force a rebuild of syscall.o.
But reading clean_dep I've convinced myself that this is a good change.
And reason to not also do it on i386? Why do we still need the MACHINE check?
Tue, Jul 23
Sat, Jul 20
I'll do this as asserts in 6 months or so if the other fixes I just pushed to -current (and Netflix's tree) eliminate all the priority messages in our logs...
Until then abandon this to de-clutter things at least a little