Re: [PATCH v3 2/2] perf x86: Exposing an Uncore unit to PMON for Intel Xeon® server platform

[Greg KH]

On Tue, Jan 14, 2020 at 04:55:03PM +0300, Sudarikov, Roman wrote:
> On 13.01.2020 17:38, Greg KH wrote:
> > On Mon, Jan 13, 2020 at 04:54:44PM +0300, roman.sudarikov@xxxxxxxxxxxxxxx wrote:
> > > From: Roman Sudarikov <roman.sudarikov@xxxxxxxxxxxxxxx>
> > > 
> > > Current version supports a server line starting Intel® Xeon® Processor
> > > Scalable Family and introduces mapping for IIO Uncore units only.
> > > Other units can be added on demand.
> > > 
> > > IIO stack to PMON mapping is exposed through:
> > >      /sys/devices/uncore_iio_<pmu_idx>/platform_mapping
> > >      in the following format: domain:bus
> > > 
> > > For example, on a 4-die Intel Xeon® server platform:
> > >      $ cat /sys/devices/uncore_iio_0/platform_mapping
> > >      0000:00,0000:40,0000:80,0000:c0
> > That's horrid to parse.  Sysfs should be one value per file, why not
> > have individual files for all of these things?
> > 
> > > Which means:
> > > IIO PMON block 0 on die 0 belongs to IIO stack on bus 0x00, domain 0x0000
> > > IIO PMON block 0 on die 1 belongs to IIO stack on bus 0x40, domain 0x0000
> > > IIO PMON block 0 on die 2 belongs to IIO stack on bus 0x80, domain 0x0000
> > > IIO PMON block 0 on die 3 belongs to IIO stack on bus 0xc0, domain 0x0000
> > Where did you get the die number from the above data?
> > 
> Mapping algorithm requires domain:bus pair for each IO stack for each die.
> Current implementation provides comma separated list of domain:bus pairs
> for each stack where offset in the list corresponds to die index.
> 
> Technically similar approach which was already implemented for the cpumask
> attribute.
> > 
> > > Co-developed-by: Alexander Antonov <alexander.antonov@xxxxxxxxx>
> > > Reviewed-by: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Alexander Antonov <alexander.antonov@xxxxxxxxx>
> > > Signed-off-by: Roman Sudarikov <roman.sudarikov@xxxxxxxxxxxxxxx>

Also, to be a bit of a pest, you all are NOT following the internal
Intel rules for submitting kernel patches.  You need a lot more reviews
before this should have "escaped" to lkml.

greg k-h