- User Since
- Jun 3 2014, 6:27 PM (207 w, 4 d)
I don't think it is a very good idea to merge two pretty much independent ZoL commits into one FreeBSD commit. It could have sense if the first one would be significantly broken and second fixed the issue, but priority upgrade implemented in second commit is only a performance optimization, mostly unrelated to scrub even, in which case having it separate would make commit history cleaner for later comparison.
Fri, May 25
Makes sense to me.
Thu, May 24
As I have told, I have no objections. I still don't like the global counters, since they mean we can never remove respective lock or at least atomics, but for now I can live with that. It is better then broken code.
Wed, May 23
Tue, May 22
Looks good to me. The only rough edge I see is that outstanding_cmds is used only for the newly added assertion, that is why it was so broken before this and nobody care.
Mon, May 21
Great to see it! I did only first on-surface look so far, so comments are mostly cosmetic.
Thu, May 17
It is OK to me. I still don't think protection against GEOM bugs belongs here, but so be it, since we any way have and acquire lock to protect that.
Wed, May 16
I agree with John that this is better then nothing, but the proper way I think would be: first suspend third-party devices, then stop clock, and then suspend timer devices, resume -- in opposite order.
Mon, May 14
The motivation for postponing the calibration was to not spend whole second on boot in cases when LAPIC timer is not used. Though that should be much less frequent lately.
Wed, May 9
Mon, May 7
Something made me to comment out hpet_disable() call on suspend 8 years ago. Wish I remember what was that.
Sat, May 5
Fri, May 4
Some more man/help tunings and so be it.
It looks good enough for me. I see some rough edges in details of SCSI interface implementation (for example, there seem to be a race window between regular and task management requests processing, submitted via different queues), but those may be addressed/improved later.
This looks like a hack, but I have no real objections.
Wed, May 2
Mon, Apr 30
Apr 25 2018
Looks mostly fine aside of commented places.
Apr 19 2018
Apr 18 2018
Apr 17 2018
Apr 16 2018
Apr 13 2018
Apr 11 2018
Is this API and device classification exist somewhere else, or being just designed? Differentiation between STRIPE, MIRROR and RAID sound somewhat odd to me, same as between SPINNING and CDROM. Besides it duplicates "GEOM::rotation_rate" attribute, we already have and use in some places (though obviously it is not passed through UFS).
Apr 9 2018
I am not closely familiar with WITNESS, so just a feeling: the long lists of blessed locks and their combinations promises high chances for them to be forgotten on following ZFS updates. At very list it would be good to document how those new mechanisms should be used.
Apr 6 2018
Apr 4 2018
Apr 3 2018
Apr 1 2018
Mar 31 2018
Mar 29 2018
Mar 28 2018
Mar 26 2018
It slightly increases lock hold time, which may be a bottleneck in case of a global lock/timer, but it probably does not worth overcomplication to avoid only that.
Mar 24 2018
I'll need to look on it closer to recall all the details, but so far my main worry about your patches is that you are increase scope of ET_HW_LOCK(), which may be a problem for SMP system with non-per-CPU event timers, where that lock is global. I remember there were cases of 8-core systems using HPET by default, where this lock was pretty busy.