Simultaneous access to same CPU/counter across processes crashes hwpmc (TSC and other classes)

Public

Hello all,

I am working on developing a new class of PMCs for the hwpmc subsystem and I ran into a kernel crash while testing my implementation. I believe I have identified the reason the crash happens, and I could try to fix it on the affected classes, but so far it's still not clear to me what model the hwpmc subsystem intends to enforce. Should simultaneous access to the same CPU and counter by different processes be allowed?

The issue happens when pmcstat is executed by two different processes on the same counter and CPU core. The affected classes contain a KASSERT() call that is triggered on a kernel built with INVARIANTS. The command below, when executed on two different shells, triggers it regardless of the CPU vendor:

pmcstat -c 0 -s tsc-cycles -w 1

While investigating the issue, I noticed that the function pmc_can_allocate_rowindex() prevents the same process from simultaneously accessing the rowindex, but this check is bypassed when a different process is doing so. Another mechanism present in hwpmc, as far as I can tell, is the PMC_PHW_FLAG_IS_SHAREABLE flag. In the Intel IAF implementation, the crash doesn't happen because it doesn't allow simultaneous access by different threads. The command below returns an error when there's already an equivalent pmcstat process running:

pmcstat -c 0 -s unhalted-core-cycles -w 1

Besides not having the affected KASSERT that is present in the affected classes, the Intel IAF implementation also doesn't set the PMC_PHW_FLAG_IS_SHAREABLE flag.

The questions are:

1 - Should the TSC and the other affected classes allow simultaneous access by different processes?
2 - If the answer is NO, I can test a patch that removes the PMC_PHW_FLAG_IS_SHAREABLE flag from the affected classes and see if this solves the problem.
3 - If the answer is YES, how can we implement simultaneous access by different processes in a safe way?

Regarding question 3, I did some tests by removing PMC_PHW_FLAG_IS_SHAREABLE from the Intel IAF implementation and adding the equivalent KASSERT(), and in this case the problem could be triggered the same way. So I believe there's nothing special about Intel IAF that avoids this from happening, other than the flag and the missing KASSERT(). That said, I also tested other events, such as uops_issued.any, and simultaneous access by different processes seemed to work fine. However, I'm not certain whether it is actually working correctly or whether something is going wrong without being detected - I didn't investigate this further for that event.

The crash happens at the KASSERT() on line 102 below. The same KASSERT() exists in other classes as well:

89 static int
90 tsc_config_pmc(int cpu, int ri, struct pmc *pm)
91 {
92         struct pmc_hw *phw;
93
94         PMCDBG3(MDP,CFG,1, "cpu=%d ri=%d pm=%p", cpu, ri, pm);
95
96         KASSERT(cpu >= 0 && cpu < pmc_cpu_max(),
97             ("[tsc,%d] illegal CPU value %d", __LINE__, cpu));
98         KASSERT(ri == 0, ("[tsc,%d] illegal row-index %d", __LINE__, ri));
99
100         phw = &tsc_pcpu[cpu]->tc_hw;
101
102         KASSERT(pm == NULL || phw->phw_pmc == NULL,
103             ("[tsc,%d] pm=%p phw->pm=%p hwpmc not unconfigured", __LINE__,
104             pm, phw->phw_pmc));
105
106         phw->phw_pmc = pm;
107
108         return (0);
109 }

So, I am writing this to seek feedback from the list on the best way to fix this crash in the affected classes. Thank you!