On Wed, 18 Jul 2018 at 15:31, Suzuki K Poulose <[email protected]> wrote: > > Hi Mathieu, > > On 07/18/2018 08:43 PM, Mathieu Poirier wrote: > > When enabling the lockdep mechanic and working with CPU-wide scenarios we > > get the following console output: > > > > > This is fixed by working with the cpu_present_mask, avoinding at the same > > the need to use get/put_online_cpus() that triggers lockdep. > > The patch looks correct to me. In fact I have a patch [1], which > does the same thing and switches to using per-cpu variable for the > paths.
Not only did you beat me to the punch but I think using a per-cpu variable is cleaner, so let's go with yours. Mathieu > > > > > Signed-off-by: Mathieu Poirier <[email protected]> > > --- > > drivers/hwtracing/coresight/coresight-etm-perf.c | 39 > > +++++++++++++----------- > > 1 file changed, 21 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c > > b/drivers/hwtracing/coresight/coresight-etm-perf.c > > index 677695635211..16b9c87d9d00 100644 > > --- a/drivers/hwtracing/coresight/coresight-etm-perf.c > > +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c > > @@ -108,26 +108,32 @@ static int etm_event_init(struct perf_event *event) > > static void free_event_data(struct work_struct *work) > > { > > int cpu; > > + void *snk_config; > > cpumask_t *mask; > > struct etm_event_data *event_data; > > struct coresight_device *sink; > > > > event_data = container_of(work, struct etm_event_data, work); > > mask = &event_data->mask; > > - /* > > - * First deal with the sink configuration. See comment in > > - * etm_setup_aux() about why we take the first available path. > > - */ > > - if (event_data->snk_config) { > > - cpu = cpumask_first(mask); > > - sink = coresight_get_sink(event_data->path[cpu]); > > - if (sink_ops(sink)->free_buffer) > > - sink_ops(sink)->free_buffer(event_data->snk_config); > > - } > > + snk_config = event_data->snk_config; > > > > for_each_cpu(cpu, mask) { > > - if (!(IS_ERR_OR_NULL(event_data->path[cpu]))) > > - coresight_release_path(event_data->path[cpu]); > > + if (IS_ERR_OR_NULL(event_data->path[cpu])) > > + continue; > > + > > + /* > > + * Free sink configuration - there can only be one sink > > + * per event. > > + */ > > + if (snk_config) { > > + sink = coresight_get_sink(event_data->path[cpu]); > > + if (sink_ops(sink)->free_buffer) { > > + sink_ops(sink)->free_buffer(snk_config); > > + snk_config = NULL; > > + } > > + } > > + > > + coresight_release_path(event_data->path[cpu]); > > } > > > > > kfree(event_data->path); > > @@ -145,16 +151,13 @@ static void *alloc_event_data(int cpu) > > if (!event_data) > > return NULL; > > > > - /* Make sure nothing disappears under us */ > > - get_online_cpus(); > > - size = num_online_cpus(); > > - > > mask = &event_data->mask; > > + size = num_present_cpus(); > > + > > if (cpu != -1) > > cpumask_set_cpu(cpu, mask); > > else > > - cpumask_copy(mask, cpu_online_mask); > > - put_online_cpus(); > > + cpumask_copy(mask, cpu_present_mask); > > I think it is safe to use cpu_online_mask outside the > get/put_online_cpus(). Using present_mask may fail as > we could fail to create a path for a CPU that is not online. > > > Please could you check if [1] fixes the problem for you ? > > [1] > http://lists.infradead.org/pipermail/linux-arm-kernel/2018-July/591606.html > > Cheers > Suzuki
