On Fri, Aug 26, 2016 at 05:44:59PM -0500, Jeremy Linton wrote:

Hi,

On 08/26/2016 10:04 AM, Punit Agrawal wrote:
(trimming)

quoted

quoted

+			pmu = kzalloc(sizeof(struct pmu_types), GFP_KERNEL);
+			if (!pmu) {
+				pr_warn("Unable to allocate pmu_types\n");
+				/*
+				 * continue to count cpus for any pmu_types
+				 * already allocated, but don't allocate any
+				 * more pmu_types. This avoids undercounting.
+				 */
+				alloc_failure = true;

Why not just fail probe and return an error? What is the benefit of
having some of the PMUs available?

AFAIC, there isn't a good reason for penalizing PMU's which we can get
working if a subset of the system PMUs can't be created. But this is per PMU
type, so with current systems the kzalloc will be called a max of 2 times
(there is the potential of a 3rd time, due to some other error handling, but
that doesn't change the argument much). AKA, this doesn't result in "partial
registration" of a PMU.

... but this will look mighty confusing to userspace, where things will
appear to "half-work", if for some reason the machine makes it that far
at all.

I think we should stick with the KISS approach and just fail the probe
as Punit is suggesting.

Will