remove cyclic and replace its uses with callout

[avg]

Summary

Sub-changes:

• callout(9): add sbt flavors of callout_schedule
• dtrace profile: make use of callout_schedule_sbt_curcpu, callout_schedule_sbt

NOTE: Previously the cyclic code would set cpu_profile_pc and cpu_profile_upc, but now we have to extract them correctly. Note that when a CPU transitions from idle state to active state cpu_activeclock() function would process timer events that might have been missed while idle and in that case there will be no interrupted frame abd thus the program counter arguments would not be set.

To do: remove solaris_cpu_t, replace the remaining used fields with DPCPU

[avg]

fix a typo in a local commit message

[gnn]

This all looks good to me. I'd like at least one other reviewer to chime in before this is committed.

[markj]

This is awesome - thanks for doing it!

sys/cddl/dev/profile/profile.c

285 ↗

(On Diff #2404)

I think you need to check whether frame == NULL here too; I was able to get a page fault with interrupts disabled here when mucking around with tick probes, and the faulting thread was the idle thread for the cpu.

480 ↗

(On Diff #2404)

Elsewhere this is written as #if defined(sun), but it's up to you.

For code under cddl/dev, I usually don't bother keeping the upstream code at all, since we don't merge to that directory from the vendor branch, and the code there tends to be fairly divergent from upstream anyway. But again, up to you.

sys/sys/callout.h

97 ↗

(On Diff #2404)

Is it actually necessary to paranthesize the macro arguments when they're being passed as separate arguments to a function call? It looks like the existing macros do it though, so callout_schedule_sbt_on might as well be consistent.

[avg]

sys/cddl/dev/profile/profile.c

285 ↗

(On Diff #2404)

Yes, thank you, will fix this.

NB: I would prefer cpu_idle() -> cpu_activeclock() path would also set td_intr_frame to something reasonable, but not sure how to do that.

480 ↗

(On Diff #2404)

I think that we are gradually switching from 'sun' to 'illumos' in new zfs code, so I applied the same approach here.

Yeah, I noticed that we keep two copies of many dtrace related files in the tree, one that is closer to the upstream and one that is actually used. I think the former is confusing and quite redundant given the vendor area. So, in this instance I decided to keep the original code for the reference.

sys/sys/callout.h

97 ↗

(On Diff #2404)

Good point. I think that the parentheses are not strictly needed here, but I'll fix the code to be consistent.

[avg]

• follow nearby style in callout_schedule_sbt family of macros
• whitespace and cosmetic changes in callout_reset family of macros
• profile_tick(): td_intr_frame could be NULL

[markj]

LGTM!

[avg]

Closed by commit rS275576 (authored by @avg).