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.

Makes sense to me.

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.

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.

Great to see it! I did only first on-surface look so far, so comments are mostly cosmetic.

Good catch.

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.

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.

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.

Something made me to comment out hpet_disable() call on suspend 8 years ago. Wish I remember what was that.

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.

Looks mostly fine aside of commented places.

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).

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.

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.

In D14827#311953, @jtl wrote:

OK, if you think this will be a problem, I'll gladly accept suggestions that maintain correctness and improve performance.

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.