Messages in this thread | | | Subject | Re: [PATCH V2 01/10] x86/CPU: Expose if cache is inclusive of lower level caches | From | Reinette Chatre <> | Date | Tue, 6 Aug 2019 09:55:56 -0700 |
| |
Hi Borislav,
On 8/6/2019 8:57 AM, Borislav Petkov wrote: > On Mon, Aug 05, 2019 at 10:57:04AM -0700, Reinette Chatre wrote: >> What do you think? > > Actually, I was thinking about something a lot simpler: something > along the lines of adding the CPUID check in a helper function which > rdt_pseudo_lock_init() calls. If the cache is not inclusive - and my > guess is it would suffice to check any cache but I'd prefer you correct > me on that - you simply return error and rdt_pseudo_lock_init() returns > early without doing any futher init. > > How does that sound?
I am a bit cautious about this. When I started this work I initially added a helper function to resctrl that calls CPUID to determine if the cache is inclusive. At that time I became aware of a discussion motivating against scattered CPUID calls and motivating for one instance of CPUID information: http://lkml.kernel.org/r/alpine.DEB.2.21.1906162141301.1760@nanos.tec.linutronix.de
My interpretation of the above resulted in a move away from calling CPUID in resctrl to the patch you are reviewing now.
I do indeed prefer a simple approach and would change to what you suggest if you find it to be best.
To answer your question about checking any cache: this seems to be different between L2 and L3. On the Atom systems where L2 pseudo-locking works well the L2 cache is not inclusive. We are also working on supporting cache pseudo-locking on L3 cache that is not inclusive.
I could add the single CPUID check during rdt_pseudo_lock_init(), checking on any CPU should work. I think it would be simpler (reasons below) to initialize that single system-wide setting in rdt_pseudo_lock_init() and keep the result locally in the pseudo-locking code, that can be referred to when the user requests the pseudo-locked region.
Simpler for two reasons: * Prevent future platform specific code within rdt_pseudo_lock_init() trying to pick when L3 is allowed to be inclusive or not. * rdt_pseudo_lock_init() does not currently make decision whether platform supports pseudo-locking - this is currently done when user requests a pseudo-lock region. rdt_pseudo_lock_init() does initialization of things that should not fail and for which resctrl's mount should not proceed if it fails. A platform not supporting pseudo-locking should not prevent resctrl from being mounted and used for cache allocation.
Thank you very much
Reinette
| |