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.1...@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
