On 09-04-20, 16:51, Sumit Gupta wrote: > We are using "read_counters_work" as local variable. So every invocation the > function will have its own copy of counters for corresponding cpu. That's > why are doing INIT_WORK_ONSTACK here. Why? To support parallel calls to reading the freq ? > > > > > > > > > + queue_work_on(cpu, read_counters_wq, &read_counters_work.work); > > > > > > > > > + flush_work(&read_counters_work.work); > > > > > > > > > > > > > > > > Why can't this be done in current context ? > > > > > > > > > > > > > > > We used work queue instead of smp_call_function_single() to have long delay. > > > > > > > > > > > > Please explain completely, you have raised more questions than you > > > > > > answered :) > > > > > > > > > > > > Why do you want to have long delays ? > > > > > > > > > > > Long delay value is used to have the observation window long enough for > > > > > correctly reconstructing the CPU frequency considering noise. > > > > > In next patch version, changed delay value to 500us which in our tests is > > > > > considered reliable. > > > > > > > > I understand that you need to put a udelay() while reading the freq from > > > > hardware, that is fine, but why do you need a workqueue for that? Why can't you > > > > just read the values directly from the same context ? > > > > > > > The register to read frequency is per core and not accessible to other > > > cores. So, we have to execute the function remotely as the target core to > > > read frequency might be different from current. > > > The functions for that are smp_call_function_single or queue_work_on. > > > We used queue_work_on() to avoid long delay inside ipi interrupt context > > > with interrupts disabled. > > > > Okay, I understand this now, finally :) > > > > But if the interrupts are disabled during some call, won't workqueues face the > > same problem ? > > > Yes, we are trying to minimize the case. But how do you know workqueues will perform better than smp_call_function_single() ? Just asking for clarity on this as normally everyone tries to do smp_call_function_single(). -- viresh
