EDAC: Drop per-memory controller buses

From: Borislav Petkov <[email protected]>

upstream 861e6ed667c83d64a42b0db41a22d6b4de4e913f commit

... and use the single edac_subsys object returned from subsys_system_register(). The idea is to have a single bus and multiple devices on it.

[Manali: backport to v4.19 -stable :
- removing per-MC bus, this enables to get rid of memory controllers
maximum number notion
- value of max number of memory controllers is 2 * MAX_NUMNODES. On two nodes system MAX_NUMNODES value is ‘1’ and so value of max number of memory controller becomes ‘2’, this patch fixes this issue when there are only 2 nodes on the system and number of memory controllers are more than ‘2’]

---
 drivers/edac/edac_mc.c       |  9 +--------
 drivers/edac/edac_mc_sysfs.c | 30 ++----------------------------
 include/linux/edac.h         |  6 ------
 3 files changed, 3 insertions(+), 42 deletions(-)

index fd440b35d76e..d899d86897d0 100644
@@ -55,8 +55,6 @@ static LIST_HEAD(mc_devices);
  */
 static const char *edac_mc_owner;
 
-static struct bus_type mc_bus[EDAC_MAX_MCS];
-
 int edac_get_report_status(void)
 {
 	return edac_report;

@@ -712,11 +710,6 @@ int edac_mc_add_mc_with_groups(struct mem_ctl_info *mci,
 	int ret = -EINVAL;
 	edac_dbg(0, "\n");
 
-	if (mci->mc_idx >= EDAC_MAX_MCS) {
-		pr_warn_once("Too many memory controllers: %d\n", mci->mc_idx);
-		return -ENODEV;
-	}
-
 #ifdef CONFIG_EDAC_DEBUG
 	if (edac_debug_level >= 3)
 		edac_mc_dump_mci(mci);

@@ -756,7 +749,7 @@ int edac_mc_add_mc_with_groups(struct mem_ctl_info *mci,
 	/* set load time so that error rate can be tracked */
 	mci->start_time = jiffies;
 
-	mci->bus = &mc_bus[mci->mc_idx];
+	mci->bus = edac_get_sysfs_subsys();
 
 	if (edac_create_sysfs_mci_device(mci, groups)) {
 		edac_mc_printk(mci, KERN_WARNING,

index d4545a9222a0..90ba228af57d 100644
@@ -920,27 +920,8 @@ static const struct device_type mci_attr_type = {
 int edac_create_sysfs_mci_device(struct mem_ctl_info *mci,
 				 const struct attribute_group **groups)
 {
-	char *name;
 	int i, err;
 
-	/*
-	 * The memory controller needs its own bus, in order to avoid
-	 * namespace conflicts at /sys/bus/edac.
-	 */
-	name = kasprintf(GFP_KERNEL, "mc%d", mci->mc_idx);
-	if (!name)
-		return -ENOMEM;
-
-	mci->bus->name = name;
-
-	edac_dbg(0, "creating bus %s\n", mci->bus->name);
-
-	err = bus_register(mci->bus);
-	if (err < 0) {
-		kfree(name);
-		return err;
-	}
-
 	/* get the /sys/devices/system/edac subsys reference */
 	mci->dev.type = &mci_attr_type;
 	device_initialize(&mci->dev);

@@ -956,7 +937,7 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info *mci,
 	err = device_add(&mci->dev);
 	if (err < 0) {
 		edac_dbg(1, "failure: create device %s\n", dev_name(&mci->dev));
-		goto fail_unregister_bus;
+		goto out;
 	}
 
 	/*

@@ -1004,10 +985,8 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info *mci,
 		device_unregister(&dimm->dev);
 	}
 	device_unregister(&mci->dev);
-fail_unregister_bus:
-	bus_unregister(mci->bus);
-	kfree(name);
 
+out:
 	return err;
 }
 

@@ -1038,13 +1017,8 @@ void edac_remove_sysfs_mci_device(struct mem_ctl_info *mci)
 
 void edac_unregister_sysfs(struct mem_ctl_info *mci)
 {
-	struct bus_type *bus = mci->bus;
-	const char *name = mci->bus->name;
-
 	edac_dbg(1, "Unregistering device %s\n", dev_name(&mci->dev));
 	device_unregister(&mci->dev);
-	bus_unregister(bus);
-	kfree(name);
 }
 
 static void mc_attr_release(struct device *dev)

index 958d69332c1d..efd5145a4c10 100644
@@ -667,10 +667,4 @@ struct mem_ctl_info {
 	bool fake_inject_ue;
 	u16 fake_inject_count;
 };
-
-/*
- * Maximum number of memory controllers in the coherent fabric.
- */
-#define EDAC_MAX_MCS	2 * MAX_NUMNODES
-
 #endif

2.19.1

Why is this a patch for the stable trees? What problem does it solve?

thanks,

greg k-h-

Hi Greg

With this patch , we are removing per-MC bus, this removes dependency on value of max number of controllers (EDAC_MAX_MCS) which is hardcoded to 2 * MAX_NUMNODES in all stable versions of kernel.

On two nodes system MAX_NUMNODES value is ‘1’ , so value of max number of memory controller becomes ‘2’, this patch fixes this issue when there are only 2 nodes on the system and number of memory controllers are more than ‘2'

https://lore.kernel.org/linux-edac/[email protected]/

-Manali

On 17/03/20, 3:52 PM, "Greg KH" <[email protected]> wrote:

On Wed, Mar 11, 2020 at 10:22:01PM -0700, Manali K Shukla wrote:
> From: Borislav Petkov <[email protected]>
>
> upstream 861e6ed667c83d64a42b0db41a22d6b4de4e913f commit
>
> ... and use the single edac_subsys object returned from
> subsys_system_register(). The idea is to have a single bus
> and multiple devices on it.
>
> Signed-off-by: Borislav Petkov <[email protected]>
> Acked-by: Mauro Carvalho Chehab <[email protected]>
> CC: Aristeu Rozanski Filho <[email protected]>
> CC: Greg KH <[email protected]>
> CC: Justin Ernst <[email protected]>
> CC: linux-edac <[email protected]>
> CC: Mauro Carvalho Chehab <[email protected]>
> CC: Russ Anderson <[email protected]>
> Cc: Tony Luck <[email protected]>
> Link: https://lkml.kernel.org/r/[email protected]
> [Manali: backport to v4.19 -stable :
> - removing per-MC bus, this enables to get rid of memory controllers
> maximum number notion
> - value of max number of memory controllers is 2 * MAX_NUMNODES. On two nodes system MAX_NUMNODES value is ‘1’ and
> so value of max number of memory controller becomes ‘2’, this patch fixes this issue when there are only 2 nodes on the system
> and number of memory controllers are more than ‘2’]
> (cherry picked from commit 861e6ed667c83d64a42b0db41a22d6b4de4e913f)
> Signed-off-by: Manali K Shukla <[email protected]>

Why is this a patch for the stable trees? What problem does it solve?

thanks,

greg k-h-

You basically repeated what you had written already.

But what is this fixing? Some platform of yours or what? Why does it need to go to stable?

Btw, please do not top-post and reply under the quoted text like all of us do.

Thx.

Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg

Certain MIPS platform can have 2 nodes and number of memory controllers can be more than '2' .

for above condition, if
#define EDAC_MAX_MCS 2 * MAX_NUMNODES,
it fails in this function edac_mc_add_mc_with_groups in below condition
if (mci->mc_idx >= EDAC_MAX_MCS) {
pr_warn_once("Too many memory controllers: %d\n", mci->mc_idx); return -ENODEV;
}
That is why this fix is needed.

    -- 

Thanks
Manali

What fix?

I see no patch in this email, nor do I see a git commit id anywhere :(

Totally confused,

greg k-h