linux-kernel - Re: [PATCH] drivers: base: cacheinfo: Ensure cpu hotplug work is done before Intel RDT

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] [day] [month] [year] [list]

Date:   Fri, 14 Jun 2019 11:21:08 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     James Morse <james.morse@....com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Reinette Chatre <reinette.chatre@...el.com>
Subject: Re: [PATCH] drivers: base: cacheinfo: Ensure cpu hotplug work is done
 before Intel RDT

On Thu, May 30, 2019 at 6:10 PM James Morse <james.morse@....com> 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 <fenghua.yu@...el.com>
> Cc: Reinette Chatre <reinette.chatre@...el.com>
> Signed-off-by: James Morse <james.morse@....com>

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: <stable@...r.kernel.org> #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
>