On Thu, May 30, 2019 at 6:10 PM James Morse <[email protected]> wrote: > > The cacheinfo structures are alloced/freed by cpu online/offline > callbacks. Originally these were only used by sysfs to expose the > cache topology to user space. Without any in-kernel dependencies > CPUHP_AP_ONLINE_DYN was an appropriate choice. > > resctrl has started using these structures to identify CPUs that > share a cache. It updates its 'domain' structures from cpu > online/offline callbacks. These depend on the cacheinfo structures > (resctrl_online_cpu()->domain_add_cpu()->get_cache_id()-> > get_cpu_cacheinfo()). > These also run as CPUHP_AP_ONLINE_DYN. > > Now that there is an in-kernel dependency, move the cacheinfo > work earlier so we know its done before resctrl's CPUHP_AP_ONLINE_DYN > work runs. > > Cc: Fenghua Yu <[email protected]> > Cc: Reinette Chatre <[email protected]> > Signed-off-by: James Morse <[email protected]>
This looks reasonable to me, but I would send the patch to Thomas Gleixner (with CCs to Tony Luck and Boris Petkov in addition to your original CC list). > --- > I haven't seen any problems because of this. If someone thinks it should > go to stable: > Cc: <[email protected]> #4.10.x > > The particular patch that added RDT is: > Fixes: 2264d9c74dda1 ("x86/intel_rdt: Build structures for each resource > based on cache topology") IMO it is OK to add a Fixes: tag if your patch is needed on top of the other on for everything to work as expected, regardless of what pieces of code are touched by each of them. That information is useful to people who need to make backporting decisions, for example (if they decide to backport the original patch, they would probably want to backport your patch on top of it). > But as this touches a different set of files, I'm not sure how appropriate > a fixes tag is. > > drivers/base/cacheinfo.c | 3 ++- > include/linux/cpuhotplug.h | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c > index a7359535caf5..b444f89a2041 100644 > --- a/drivers/base/cacheinfo.c > +++ b/drivers/base/cacheinfo.c > @@ -655,7 +655,8 @@ static int cacheinfo_cpu_pre_down(unsigned int cpu) > > static int __init cacheinfo_sysfs_init(void) > { > - return cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "base/cacheinfo:online", > + return cpuhp_setup_state(CPUHP_AP_BASE_CACHEINFO_ONLINE, > + "base/cacheinfo:online", > cacheinfo_cpu_online, > cacheinfo_cpu_pre_down); > } > device_initcall(cacheinfo_sysfs_init); > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h > index 6a381594608c..50c893f03c21 100644 > --- a/include/linux/cpuhotplug.h > +++ b/include/linux/cpuhotplug.h > @@ -175,6 +175,7 @@ enum cpuhp_state { > CPUHP_AP_WATCHDOG_ONLINE, > CPUHP_AP_WORKQUEUE_ONLINE, > CPUHP_AP_RCUTREE_ONLINE, > + CPUHP_AP_BASE_CACHEINFO_ONLINE, > CPUHP_AP_ONLINE_DYN, > CPUHP_AP_ONLINE_DYN_END = CPUHP_AP_ONLINE_DYN + 30, > CPUHP_AP_X86_HPET_ONLINE, > -- > 2.20.1 >
