From: Robert Richter <rrichter@marvell.com> To: Borislav Petkov <bp@alien8.de>, Mauro Carvalho Chehab <mchehab@kernel.org>, Tony Luck <tony.luck@intel.com> Cc: James Morse <james.morse@arm.com>, Aristeu Rozanski <aris@redhat.com>, Robert Richter <rrichter@marvell.com>, Matthias Brugger <mbrugger@suse.com>, <linux-edac@vger.kernel.org>, <linux-kernel@vger.kernel.org> Subject: [PATCH v2 08/10] EDAC/ghes: Carve out MC device handling into separate functions Date: Wed, 22 Apr 2020 13:58:12 +0200 Message-ID: <20200422115814.22205-9-rrichter@marvell.com> (raw) In-Reply-To: <20200422115814.22205-1-rrichter@marvell.com> The functions are too long, carve out code that handles MC devices into the new functions ghes_mc_create(), ghes_mc_add_or_free() and ghes_mc_free(). Apart from better code readability the functions can be reused and the implementation of the error paths becomes easier. Signed-off-by: Robert Richter <rrichter@marvell.com> --- drivers/edac/ghes_edac.c | 141 +++++++++++++++++++++++---------------- 1 file changed, 83 insertions(+), 58 deletions(-) diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c index 4eadc5b344c8..af0a769071f4 100644 --- a/drivers/edac/ghes_edac.c +++ b/drivers/edac/ghes_edac.c @@ -535,16 +535,88 @@ static struct acpi_platform_list plat_list[] = { { } /* End */ }; -int ghes_edac_register(struct ghes *ghes, struct device *dev) +static struct mem_ctl_info *ghes_mc_create(struct device *dev, int mc_idx, + int num_dimm) { - bool fake = false; - int rc = 0, num_dimm = 0; + struct edac_mc_layer layers[1]; struct mem_ctl_info *mci; struct ghes_mci *pvt; - struct edac_mc_layer layers[1]; - struct dimm_fill dimm_fill; + + layers[0].type = EDAC_MC_LAYER_ALL_MEM; + layers[0].size = num_dimm; + layers[0].is_virt_csrow = true; + + mci = edac_mc_alloc(mc_idx, ARRAY_SIZE(layers), layers, sizeof(*pvt)); + if (!mci) + return NULL; + + pvt = mci->pvt_info; + pvt->mci = mci; + + mci->pdev = dev; + mci->mtype_cap = MEM_FLAG_EMPTY; + mci->edac_ctl_cap = EDAC_FLAG_NONE; + mci->edac_cap = EDAC_FLAG_NONE; + mci->mod_name = "ghes_edac.c"; + mci->ctl_name = "ghes_edac"; + mci->dev_name = "ghes"; + + return mci; +} + +static int ghes_mc_add_or_free(struct mem_ctl_info *mci, + struct list_head *dimm_list) +{ unsigned long flags; - int idx = -1; + int rc; + + rc = edac_mc_add_mc(mci); + if (rc < 0) { + ghes_dimm_release(dimm_list); + edac_mc_free(mci); + return rc; + } + + spin_lock_irqsave(&ghes_lock, flags); + ghes_pvt = mci->pvt_info; + list_splice_tail(dimm_list, &ghes_dimm_list); + spin_unlock_irqrestore(&ghes_lock, flags); + + return 0; +} + +static void ghes_mc_free(void) +{ + struct mem_ctl_info *mci; + unsigned long flags; + LIST_HEAD(dimm_list); + + /* + * Wait for the irq handler being finished. + */ + spin_lock_irqsave(&ghes_lock, flags); + mci = ghes_pvt ? ghes_pvt->mci : NULL; + ghes_pvt = NULL; + list_splice_init(&ghes_dimm_list, &dimm_list); + spin_unlock_irqrestore(&ghes_lock, flags); + + ghes_dimm_release(&dimm_list); + + if (!mci) + return; + + mci = edac_mc_del_mc(mci->pdev); + if (mci) + edac_mc_free(mci); +} + +int ghes_edac_register(struct ghes *ghes, struct device *dev) +{ + struct dimm_fill dimm_fill; + int rc = 0, num_dimm = 0; + struct mem_ctl_info *mci; + bool fake = false; + int idx; if (IS_ENABLED(CONFIG_X86)) { /* Check if safe to enable on this system */ @@ -577,27 +649,12 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev) num_dimm = 1; } - layers[0].type = EDAC_MC_LAYER_ALL_MEM; - layers[0].size = num_dimm; - layers[0].is_virt_csrow = true; - - mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(*pvt)); + mci = ghes_mc_create(dev, 0, num_dimm); if (!mci) { rc = -ENOMEM; goto unlock; } - pvt = mci->pvt_info; - pvt->mci = mci; - - mci->pdev = dev; - mci->mtype_cap = MEM_FLAG_EMPTY; - mci->edac_ctl_cap = EDAC_FLAG_NONE; - mci->edac_cap = EDAC_FLAG_NONE; - mci->mod_name = "ghes_edac.c"; - mci->ctl_name = "ghes_edac"; - mci->dev_name = "ghes"; - if (fake) { pr_info("This system has a very crappy BIOS: It doesn't even list the DIMMS.\n"); pr_info("Its SMBIOS info is wrong. It is doubtful that the error report would\n"); @@ -627,18 +684,9 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev) dimm->edac_mode = EDAC_SECDED; } - rc = edac_mc_add_mc(mci); - if (rc < 0) { - ghes_dimm_release(&dimm_fill.dimms); - edac_mc_free(mci); - rc = -ENODEV; + rc = ghes_mc_add_or_free(mci, &dimm_fill.dimms); + if (rc < 0) goto unlock; - } - - spin_lock_irqsave(&ghes_lock, flags); - ghes_pvt = pvt; - list_splice_tail(&dimm_fill.dimms, &ghes_dimm_list); - spin_unlock_irqrestore(&ghes_lock, flags); /* only set on success */ refcount_set(&ghes_refcount, 1); @@ -656,35 +704,12 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev) void ghes_edac_unregister(struct ghes *ghes) { - struct mem_ctl_info *mci; - unsigned long flags; - LIST_HEAD(dimm_list); - mutex_lock(&ghes_reg_mutex); - if (!refcount_dec_and_test(&ghes_refcount)) - goto unlock; - - /* - * Wait for the irq handler being finished. - */ - spin_lock_irqsave(&ghes_lock, flags); - mci = ghes_pvt ? ghes_pvt->mci : NULL; - ghes_pvt = NULL; - list_splice_init(&ghes_dimm_list, &dimm_list); - spin_unlock_irqrestore(&ghes_lock, flags); - - ghes_dimm_release(&dimm_list); - - if (!mci) - goto unlock; - - mci = edac_mc_del_mc(mci->pdev); - if (mci) { - edac_mc_free(mci); + if (refcount_dec_and_test(&ghes_refcount)) { + ghes_mc_free(); ghes_dimm_pool_destroy(); } -unlock: mutex_unlock(&ghes_reg_mutex); } -- 2.20.1
next prev parent reply index Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-04-22 11:58 [PATCH v2 00/10] EDAC/mc/ghes: Fixes, cleanup and reworks Robert Richter 2020-04-22 11:58 ` [PATCH v2 01/10] EDAC/mc: Fix usage of snprintf() and dimm location setup Robert Richter 2020-04-22 20:52 ` Borislav Petkov 2020-05-19 9:27 ` Robert Richter 2020-04-22 11:58 ` [PATCH v2 02/10] EDAC/mc: Use int type for parameters of edac_mc_alloc() Robert Richter 2020-04-23 17:49 ` Borislav Petkov 2020-05-19 9:33 ` Robert Richter 2020-04-22 11:58 ` [PATCH v2 03/10] EDAC/ghes: Remove unused members of struct ghes_edac_pvt, rename it to ghes_mci Robert Richter 2020-04-23 17:55 ` Borislav Petkov 2020-05-05 7:50 ` Robert Richter 2020-04-22 11:58 ` [PATCH v2 04/10] EDAC/ghes: Make SMBIOS handle private data to ghes Robert Richter 2020-04-24 12:12 ` kbuild test robot 2020-04-24 16:21 ` Borislav Petkov 2020-05-05 12:48 ` Robert Richter 2020-04-22 11:58 ` [PATCH v2 05/10] EDAC/ghes: Setup DIMM label from DMI and use it in error reports Robert Richter 2020-04-22 11:58 ` [PATCH v2 06/10] EDAC/ghes: Remove local variable rdr_mask in ghes_edac_dmidecode() Robert Richter 2020-04-27 7:08 ` Borislav Petkov 2020-04-27 17:24 ` Luck, Tony 2020-04-27 17:34 ` Borislav Petkov 2020-05-19 9:34 ` Robert Richter 2020-04-22 11:58 ` [PATCH v2 07/10] EDAC/ghes: Cleanup struct ghes_edac_dimm_fill, rename it to ghes_dimm_fill Robert Richter 2020-04-27 14:00 ` Borislav Petkov 2020-05-19 9:35 ` Robert Richter 2020-04-22 11:58 ` Robert Richter [this message] 2020-04-27 16:38 ` [PATCH v2 08/10] EDAC/ghes: Carve out MC device handling into separate functions Borislav Petkov 2020-05-06 8:45 ` Robert Richter 2020-05-11 13:32 ` Borislav Petkov 2020-05-19 9:57 ` Robert Richter 2020-04-22 11:58 ` [PATCH v2 09/10] EDAC/ghes: Have a separate code path for creating the fake MC Robert Richter 2020-04-22 11:58 ` [PATCH v2 10/10] EDAC/ghes: Carve out code into ghes_edac_register_{one,fake}() Robert Richter 2020-05-06 8:53 ` [PATCH v2 00/10] EDAC/mc/ghes: Fixes, cleanup and reworks Robert Richter
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200422115814.22205-9-rrichter@marvell.com \ --to=rrichter@marvell.com \ --cc=aris@redhat.com \ --cc=bp@alien8.de \ --cc=james.morse@arm.com \ --cc=linux-edac@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mbrugger@suse.com \ --cc=mchehab@kernel.org \ --cc=tony.luck@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Linux-EDAC Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-edac/0 linux-edac/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-edac linux-edac/ https://lore.kernel.org/linux-edac \ linux-edac@vger.kernel.org public-inbox-index linux-edac Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-edac AGPL code for this site: git clone https://public-inbox.org/public-inbox.git