diff mbox series

[stable,v4.19] EDAC: Drop per-memory controller buses

Message ID 20200312052201.49456-1-manashuk@cisco.com (mailing list archive)
State New, archived
Headers show
Series [stable,v4.19] EDAC: Drop per-memory controller buses | expand

Commit Message

Manali Shukla (manashuk) March 12, 2020, 5:22 a.m. UTC 
From: Borislav Petkov <bp@suse.de>

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 <bp@suse.de>
Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
CC: Aristeu Rozanski Filho <arozansk@redhat.com>
CC: Greg KH <gregkh@linuxfoundation.org>
CC: Justin Ernst <justin.ernst@hpe.com>
CC: linux-edac <linux-edac@vger.kernel.org>
CC: Mauro Carvalho Chehab <mchehab@kernel.org>
CC: Russ Anderson <rja@hpe.com>
Cc: Tony Luck <tony.luck@intel.com>
Link: https://lkml.kernel.org/r/20180926152752.GG5584@zn.tnic
[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 <manashuk@cisco.com>
---
 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(-)

Comments

gregkh@linuxfoundation.org March 17, 2020, 10:22 a.m. UTC | #1 
On Wed, Mar 11, 2020 at 10:22:01PM -0700, Manali K Shukla wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> 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 <bp@suse.de>
> Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> CC: Aristeu Rozanski Filho <arozansk@redhat.com>
> CC: Greg KH <gregkh@linuxfoundation.org>
> CC: Justin Ernst <justin.ernst@hpe.com>
> CC: linux-edac <linux-edac@vger.kernel.org>
> CC: Mauro Carvalho Chehab <mchehab@kernel.org>
> CC: Russ Anderson <rja@hpe.com>
> Cc: Tony Luck <tony.luck@intel.com>
> Link: https://lkml.kernel.org/r/20180926152752.GG5584@zn.tnic
> [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 <manashuk@cisco.com>

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

thanks,

greg k-h-
Manali Shukla (manashuk) April 5, 2020, 6:01 p.m. UTC | #2 
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/20180926152752.GG5584@zn.tnic/

-Manali

On 17/03/20, 3:52 PM, "Greg KH" <gregkh@linuxfoundation.org> wrote:

    On Wed, Mar 11, 2020 at 10:22:01PM -0700, Manali K Shukla wrote:
    > From: Borislav Petkov <bp@suse.de>
    > 
    > 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 <bp@suse.de>
    > Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
    > CC: Aristeu Rozanski Filho <arozansk@redhat.com>
    > CC: Greg KH <gregkh@linuxfoundation.org>
    > CC: Justin Ernst <justin.ernst@hpe.com>
    > CC: linux-edac <linux-edac@vger.kernel.org>
    > CC: Mauro Carvalho Chehab <mchehab@kernel.org>
    > CC: Russ Anderson <rja@hpe.com>
    > Cc: Tony Luck <tony.luck@intel.com>
    > Link: https://lkml.kernel.org/r/20180926152752.GG5584@zn.tnic
    > [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 <manashuk@cisco.com>
    
    Why is this a patch for the stable trees?  What problem does it solve?
    
    thanks,
    
    greg k-h-
Borislav Petkov April 5, 2020, 6:06 p.m. UTC | #3 
On Sun, Apr 05, 2020 at 06:01:21PM +0000, Manali Shukla (manashuk) wrote:
> 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'

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.
Manali Shukla (manashuk) April 5, 2020, 7:26 p.m. UTC | #4 
> On 05/04/20, 11:36 PM, "Borislav Petkov" <bp@suse.de> wrote:

>    On Sun, Apr 05, 2020 at 06:01:21PM +0000, Manali Shukla (manashuk) wrote:
>> 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'
    
 > 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?
    
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.

 >  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

Thanks 
Manali
gregkh@linuxfoundation.org June 16, 2020, 10:37 a.m. UTC | #5 
On Sun, Apr 05, 2020 at 07:26:01PM +0000, Manali Shukla (manashuk) wrote:
> > On 05/04/20, 11:36 PM, "Borislav Petkov" <bp@suse.de> wrote:
> 
> >    On Sun, Apr 05, 2020 at 06:01:21PM +0000, Manali Shukla (manashuk) wrote:
> >> 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'
>     
>  > 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?
>     
> 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.

What fix?

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

Totally confused,

greg k-h
diff mbox series

Patch

diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index fd440b35d76e..d899d86897d0 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -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,
diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index d4545a9222a0..90ba228af57d 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -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)
diff --git a/include/linux/edac.h b/include/linux/edac.h
index 958d69332c1d..efd5145a4c10 100644
--- a/include/linux/edac.h
+++ b/include/linux/edac.h
@@ -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