Em Wed, 26 Sep 2018 17:27:52 +0200 Borislav Petkov <b...@alien8.de> escreveu:
> On Wed, Sep 26, 2018 at 11:35:11AM +0200, Borislav Petkov wrote: > > * or Greg coming and saying, you're using bus_type all wrong and you > > shouldn't and you should remove it completely! :-) > > Yap, and so he did! :-) > > It looks like we can remove the whole per-MC bus thing, see below. I guess this is/was needed to create things like this: lrwxrwxrwx 1 root root 0 set 26 05:24 /sys/bus/edac/devices/mc -> ../../../devices/system/edac/mc (I don't test this logic for a while). > Patch > seems to work here and grepping sysfs hierarchy before and after: > > find /sys/ | grep -i edac > ... > > doesn't show any differences. > > Tony, I'd appreciate running this on one of your big boxes to check > nothing is missing in sysfs... There are a few EDAC drivers for old server chipsets that create error report mechanisms for PCI bus controllers. I don't remember the specifics anymore. I *suspect* those are the drivers that do such usage: $ git grep -l edac_pci_add_device drivers/edac/amd8111_edac.c drivers/edac/amd8131_edac.c drivers/edac/edac_pci.c drivers/edac/edac_pci.h drivers/edac/mpc85xx_edac.c drivers/edac/mv64x60_edac.c drivers/edac/octeon_edac-pci.c None of them use Intel chipsets. Last time I tried to test it (more than 5 years ago), it was hard to find such systems at the labs I had access on that time. Anyway, as this is a more unusual usage of EDAC API, it could be worth to double-check if this patch won't break it, maybe doing some test on such machines before/after this patch in order to verify that this won't cause regressions there. In thesis, it shouldn't affect, as the changes are happening at edac_mc*.c, but I won't doubt that it might have a hidden dependency somewhere. > > Thx. > > --- > 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(-) > > diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c > index 7d3edd713932..13594ffadcb3 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; > @@ -716,11 +714,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); > @@ -760,7 +753,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 20374b8248f0..2ca2012f2857 100644 > --- a/drivers/edac/edac_mc_sysfs.c > +++ b/drivers/edac/edac_mc_sysfs.c > @@ -914,27 +914,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); > @@ -950,7 +931,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; > } > > /* > @@ -998,10 +979,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; > } > > @@ -1032,13 +1011,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 a45ce1f84bfc..dd8d006f1837 100644 > --- a/include/linux/edac.h > +++ b/include/linux/edac.h > @@ -668,10 +668,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 16 > - > #endif Thanks, Mauro