linux-kernel - Re: [PATCH v3 1/2] perf x86: Infrastructure for exposing an Uncore unit to PMON mapping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200114133958.GE170376@krava>
Date:   Tue, 14 Jan 2020 14:39:58 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     "Sudarikov, Roman" <roman.sudarikov@...ux.intel.com>
Cc:     Greg KH <gregkh@...uxfoundation.org>, peterz@...radead.org,
        mingo@...hat.com, acme@...nel.org, mark.rutland@....com,
        alexander.shishkin@...ux.intel.com, namhyung@...nel.org,
        linux-kernel@...r.kernel.org, eranian@...gle.com,
        bgregg@...flix.com, ak@...ux.intel.com, kan.liang@...ux.intel.com,
        alexander.antonov@...el.com
Subject: Re: [PATCH v3 1/2] perf x86: Infrastructure for exposing an Uncore
 unit to PMON mapping

On Tue, Jan 14, 2020 at 04:24:34PM +0300, Sudarikov, Roman wrote:

SNIP

> > >   {
> > >   	struct intel_uncore_pmu *pmus;
> > > @@ -950,10 +976,19 @@ static int __init uncore_type_init(struct intel_uncore_type *type, bool setid)
> > >   			attr_group->attrs[j] = &type->event_descs[j].attr.attr;
> > >   		type->events_group = &attr_group->group;
> > > -	}
> > > +	} else
> > > +		type->events_group = &empty_group;
> > Why???
> Hi Greg,
> 
> Technically, what I'm trying to do is to add an attribute which depends on
> the uncore pmu type and BIOS support. New attribute is added to the end of
> the attribute groups array. It appears that the events attribute group is
> optional for most of the uncore pmus for x86/intel, i.e. events_group =
> NULL.
> 
> NULL element in the middle of the attribute groups array "hides" all others
> attribute groups which follows that element.
> 
> To work around it, embedded NULL elements should be either removed from
> the attribute groups array [1] or replaced with empty attribute; see
> implementation above.
> 
> If both approaches are incorrect then please advice what would be correct
> solution for that case.

hi,
I think Greg is reffering to the recent cleanup where we used attribute
groups with is_vissible callbacks, you can check changes below:

b7c9b3927337 perf/x86/intel: Use ->is_visible callback for default group
6a9f4efe78af perf/x86: Use update attribute groups for default attributes
b657688069a2 perf/x86/intel: Use update attributes for skylake format
3ea40ac77261 perf/x86: Use update attribute groups for extra format
1f157286829c perf/x86: Use update attribute groups for caps
baa0c83363c7 perf/x86: Use the new pmu::update_attrs attribute group

jirka

> 
> [1] https://lore.kernel.org/lkml/20191210091451.6054-3-roman.sudarikov@linux.intel.com/
> 
> Thanks,
> Roman
> > Didn't we fix up the x86 attributes to work properly and not mess around
> > with trying to merge groups and the like?  Please don't perpetuate that
> > more...
> > 
> > thanks,
> > 
> > greg k-h
> 
>