linux-kernel - Re: [PATCH 5/6] perf/core: Use ioctl to communicate driver configuration to kernel

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]

Date:   Tue, 3 Jul 2018 13:56:52 +0300
From:   Alexander Shishkin <alexander.shishkin@...ux.intel.com>
To:     Mathieu Poirier <mathieu.poirier@...aro.org>, peterz@...radead.org,
        acme@...nel.org, mingo@...hat.com, tglx@...utronix.de,
        alexander.shishkin@...ux.intel.com, schwidefsky@...ibm.com,
        heiko.carstens@...ibm.com, will.deacon@....com,
        mark.rutland@....com, jolsa@...hat.com, namhyung@...nel.org,
        adrian.hunter@...el.com, ast@...nel.org,
        gregkh@...uxfoundation.org, hpa@...or.com,
        linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 5/6] perf/core: Use ioctl to communicate driver
 configuration to kernel

On Tue, Jul 03, 2018 at 01:03:48PM +0300, Alexander Shishkin wrote:
> On Mon, Jul 02, 2018 at 04:33:29PM -0600, Mathieu Poirier wrote:
> > +/*
> > + * PMU driver configuration works the same way as filter management above,
> > + * but without the need to deal with memory mapping.  Driver configuration
> > + * arrives through the SET_DRV_CONFIG ioctl() where it is validated and applied
> > + * to the event.  When the PMU is ready it calls perf_event_drv_config_sync() to
> > + * bring the configuration information within reach of the PMU.
> 
> Wait a second. The reason why we dance around with the generations of filters
> is the locking order of ctx::mutex vs mmap_sem. In an mmap path, where we're
> notified about mapping changes, we're called under the latter, and we'd need
> to grab the former to update the event configuration. In your case, the
> update comes in via perf_ioctl(), where we're already holding the ctx::mutex,
> so you can just kick the PMU right there, via an event_function_call() or
> perf_event_stop(restart=1). In the latter case, your pmu::start() would just
> grab the new configuration. Should also be about 90% less code. :)

Also, since it affects the AUX buffer configuration, it is probably a one
time ioctl command that you issue before you mmap the buffer. If that's
the case, you don't even have to worry about stopping the event, as it
shouldn't be running, because without the buffer perf_aux_output_begin()
should fail and so should the pmu::add() iirc.

Regards,
--
Alex