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
