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]
Message-ID: <d6ff1070-4f60-4699-bdc4-11bd72249d96@intel.com>
Date: Mon, 5 Aug 2024 08:40:22 -0700
From: Dave Jiang <dave.jiang@...el.com>
To: kan.liang@...ux.intel.com, peterz@...radead.org, mingo@...hat.com,
 acme@...nel.org, namhyung@...nel.org, irogers@...gle.com,
 linux-kernel@...r.kernel.org
Cc: Fenghua Yu <fenghua.yu@...el.com>, Vinod Koul <vkoul@...nel.org>,
 dmaengine@...r.kernel.org, "Zanussi, Tom" <tom.zanussi@...el.com>
Subject: Re: [PATCH 5/7] dmaengine: idxd: Clean up cpumask and hotplug for
 perfmon



On 8/2/24 8:16 AM, kan.liang@...ux.intel.com wrote:
> From: Kan Liang <kan.liang@...ux.intel.com>
> 
> The idxd PMU is system-wide scope, which is supported by the generic
> perf_event subsystem now.
> 
> Set the scope for the idxd PMU and remove all the cpumask and hotplug
> codes.
> 
> Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
> Cc: Fenghua Yu <fenghua.yu@...el.com>
> Cc: Dave Jiang <dave.jiang@...el.com>
> Cc: Vinod Koul <vkoul@...nel.org>
> Cc: dmaengine@...r.kernel.org

Cc: Tom Zanussi <tom.zanussi@...el.com>

Reviewed-by: Dave Jiang <dave.jiang@...el.com>
> ---
>  drivers/dma/idxd/idxd.h    |  7 ---
>  drivers/dma/idxd/init.c    |  3 --
>  drivers/dma/idxd/perfmon.c | 98 +-------------------------------------
>  3 files changed, 1 insertion(+), 107 deletions(-)
> 
> diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
> index 868b724a3b75..d84e21daa991 100644
> --- a/drivers/dma/idxd/idxd.h
> +++ b/drivers/dma/idxd/idxd.h
> @@ -124,7 +124,6 @@ struct idxd_pmu {
>  
>  	struct pmu pmu;
>  	char name[IDXD_NAME_SIZE];
> -	int cpu;
>  
>  	int n_counters;
>  	int counter_width;
> @@ -135,8 +134,6 @@ struct idxd_pmu {
>  
>  	unsigned long supported_filters;
>  	int n_filters;
> -
> -	struct hlist_node cpuhp_node;
>  };
>  
>  #define IDXD_MAX_PRIORITY	0xf
> @@ -803,14 +800,10 @@ void idxd_user_counter_increment(struct idxd_wq *wq, u32 pasid, int index);
>  int perfmon_pmu_init(struct idxd_device *idxd);
>  void perfmon_pmu_remove(struct idxd_device *idxd);
>  void perfmon_counter_overflow(struct idxd_device *idxd);
> -void perfmon_init(void);
> -void perfmon_exit(void);
>  #else
>  static inline int perfmon_pmu_init(struct idxd_device *idxd) { return 0; }
>  static inline void perfmon_pmu_remove(struct idxd_device *idxd) {}
>  static inline void perfmon_counter_overflow(struct idxd_device *idxd) {}
> -static inline void perfmon_init(void) {}
> -static inline void perfmon_exit(void) {}
>  #endif
>  
>  /* debugfs */
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index 21f6905b554d..5725ea82c409 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -878,8 +878,6 @@ static int __init idxd_init_module(void)
>  	else
>  		support_enqcmd = true;
>  
> -	perfmon_init();
> -
>  	err = idxd_driver_register(&idxd_drv);
>  	if (err < 0)
>  		goto err_idxd_driver_register;
> @@ -928,7 +926,6 @@ static void __exit idxd_exit_module(void)
>  	idxd_driver_unregister(&idxd_drv);
>  	pci_unregister_driver(&idxd_pci_driver);
>  	idxd_cdev_remove();
> -	perfmon_exit();
>  	idxd_remove_debugfs();
>  }
>  module_exit(idxd_exit_module);
> diff --git a/drivers/dma/idxd/perfmon.c b/drivers/dma/idxd/perfmon.c
> index 5e94247e1ea7..f511cf15845b 100644
> --- a/drivers/dma/idxd/perfmon.c
> +++ b/drivers/dma/idxd/perfmon.c
> @@ -6,29 +6,6 @@
>  #include "idxd.h"
>  #include "perfmon.h"
>  
> -static ssize_t cpumask_show(struct device *dev, struct device_attribute *attr,
> -			    char *buf);
> -
> -static cpumask_t		perfmon_dsa_cpu_mask;
> -static bool			cpuhp_set_up;
> -static enum cpuhp_state		cpuhp_slot;
> -
> -/*
> - * perf userspace reads this attribute to determine which cpus to open
> - * counters on.  It's connected to perfmon_dsa_cpu_mask, which is
> - * maintained by the cpu hotplug handlers.
> - */
> -static DEVICE_ATTR_RO(cpumask);
> -
> -static struct attribute *perfmon_cpumask_attrs[] = {
> -	&dev_attr_cpumask.attr,
> -	NULL,
> -};
> -
> -static struct attribute_group cpumask_attr_group = {
> -	.attrs = perfmon_cpumask_attrs,
> -};
> -
>  /*
>   * These attributes specify the bits in the config word that the perf
>   * syscall uses to pass the event ids and categories to perfmon.
> @@ -67,16 +44,9 @@ static struct attribute_group perfmon_format_attr_group = {
>  
>  static const struct attribute_group *perfmon_attr_groups[] = {
>  	&perfmon_format_attr_group,
> -	&cpumask_attr_group,
>  	NULL,
>  };
>  
> -static ssize_t cpumask_show(struct device *dev, struct device_attribute *attr,
> -			    char *buf)
> -{
> -	return cpumap_print_to_pagebuf(true, buf, &perfmon_dsa_cpu_mask);
> -}
> -
>  static bool is_idxd_event(struct idxd_pmu *idxd_pmu, struct perf_event *event)
>  {
>  	return &idxd_pmu->pmu == event->pmu;
> @@ -217,7 +187,6 @@ static int perfmon_pmu_event_init(struct perf_event *event)
>  		return -EINVAL;
>  
>  	event->hw.event_base = ioread64(PERFMON_TABLE_OFFSET(idxd));
> -	event->cpu = idxd->idxd_pmu->cpu;
>  	event->hw.config = event->attr.config;
>  
>  	if (event->group_leader != event)
> @@ -488,6 +457,7 @@ static void idxd_pmu_init(struct idxd_pmu *idxd_pmu)
>  	idxd_pmu->pmu.stop		= perfmon_pmu_event_stop;
>  	idxd_pmu->pmu.read		= perfmon_pmu_event_update;
>  	idxd_pmu->pmu.capabilities	= PERF_PMU_CAP_NO_EXCLUDE;
> +	idxd_pmu->pmu.scope		= PERF_PMU_SCOPE_SYS_WIDE;
>  	idxd_pmu->pmu.module		= THIS_MODULE;
>  }
>  
> @@ -496,59 +466,17 @@ void perfmon_pmu_remove(struct idxd_device *idxd)
>  	if (!idxd->idxd_pmu)
>  		return;
>  
> -	cpuhp_state_remove_instance(cpuhp_slot, &idxd->idxd_pmu->cpuhp_node);
>  	perf_pmu_unregister(&idxd->idxd_pmu->pmu);
>  	kfree(idxd->idxd_pmu);
>  	idxd->idxd_pmu = NULL;
>  }
>  
> -static int perf_event_cpu_online(unsigned int cpu, struct hlist_node *node)
> -{
> -	struct idxd_pmu *idxd_pmu;
> -
> -	idxd_pmu = hlist_entry_safe(node, typeof(*idxd_pmu), cpuhp_node);
> -
> -	/* select the first online CPU as the designated reader */
> -	if (cpumask_empty(&perfmon_dsa_cpu_mask)) {
> -		cpumask_set_cpu(cpu, &perfmon_dsa_cpu_mask);
> -		idxd_pmu->cpu = cpu;
> -	}
> -
> -	return 0;
> -}
> -
> -static int perf_event_cpu_offline(unsigned int cpu, struct hlist_node *node)
> -{
> -	struct idxd_pmu *idxd_pmu;
> -	unsigned int target;
> -
> -	idxd_pmu = hlist_entry_safe(node, typeof(*idxd_pmu), cpuhp_node);
> -
> -	if (!cpumask_test_and_clear_cpu(cpu, &perfmon_dsa_cpu_mask))
> -		return 0;
> -
> -	target = cpumask_any_but(cpu_online_mask, cpu);
> -	/* migrate events if there is a valid target */
> -	if (target < nr_cpu_ids) {
> -		cpumask_set_cpu(target, &perfmon_dsa_cpu_mask);
> -		perf_pmu_migrate_context(&idxd_pmu->pmu, cpu, target);
> -	}
> -
> -	return 0;
> -}
> -
>  int perfmon_pmu_init(struct idxd_device *idxd)
>  {
>  	union idxd_perfcap perfcap;
>  	struct idxd_pmu *idxd_pmu;
>  	int rc = -ENODEV;
>  
> -	/*
> -	 * perfmon module initialization failed, nothing to do
> -	 */
> -	if (!cpuhp_set_up)
> -		return -ENODEV;
> -
>  	/*
>  	 * If perfmon_offset or num_counters is 0, it means perfmon is
>  	 * not supported on this hardware.
> @@ -624,11 +552,6 @@ int perfmon_pmu_init(struct idxd_device *idxd)
>  	if (rc)
>  		goto free;
>  
> -	rc = cpuhp_state_add_instance(cpuhp_slot, &idxd_pmu->cpuhp_node);
> -	if (rc) {
> -		perf_pmu_unregister(&idxd->idxd_pmu->pmu);
> -		goto free;
> -	}
>  out:
>  	return rc;
>  free:
> @@ -637,22 +560,3 @@ int perfmon_pmu_init(struct idxd_device *idxd)
>  
>  	goto out;
>  }
> -
> -void __init perfmon_init(void)
> -{
> -	int rc = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
> -					 "driver/dma/idxd/perf:online",
> -					 perf_event_cpu_online,
> -					 perf_event_cpu_offline);
> -	if (WARN_ON(rc < 0))
> -		return;
> -
> -	cpuhp_slot = rc;
> -	cpuhp_set_up = true;
> -}
> -
> -void __exit perfmon_exit(void)
> -{
> -	if (cpuhp_set_up)
> -		cpuhp_remove_multi_state(cpuhp_slot);
> -}

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ