All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Henrique Barboza <danielhb413@gmail.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: Daniel Henrique Barboza <danielhb413@gmail.com>,
	david@gibson.dropbear.id.au
Subject: [PATCH 3/3] pseries/hotplug-memory.c: adding dlpar_memory_remove_lmbs_common()
Date: Fri, 30 Apr 2021 09:09:17 -0300	[thread overview]
Message-ID: <20210430120917.217951-4-danielhb413@gmail.com> (raw)
In-Reply-To: <20210430120917.217951-1-danielhb413@gmail.com>

One difference between dlpar_memory_remove_by_count() and
dlpar_memory_remove_by_ic() is that the latter, added in commit
753843471cbb, removes the LMBs in a contiguous block. This was done
because QEMU works with DIMMs, which is nothing more than a set of LMBs,
that must be added or removed together. Failing to remove one LMB must
fail the removal of the entire set of LMBs. Another difference is that
dlpar_memory_remove_by_ic() is going to set the LMB DRC to unisolate in
case of a removal error, which is a no-op for the hypervisors that
doesn't care about this error handling knob, and could be called by
remove_by_count() without issues.

Aside from that, the logic is the same for both functions, and yet we
keep them separated and having to duplicate LMB removal logic in both.

This patch introduces a helper called dlpar_memory_remove_lmbs_common()
to be used by both functions. The helper handles the case of block
removal of remove_by_ic() by failing earlier in the validation and
removal steps if the helper was called by remove_by_ic() (i.e. it was
called with a drc_index) while preserving the more relaxed behavior of
remove_by_count() if drc_index is 0.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 .../platforms/pseries/hotplug-memory.c        | 163 +++++++-----------
 1 file changed, 67 insertions(+), 96 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 4e6d162c3f1a..a031993725ca 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -399,25 +399,43 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
 	return 0;
 }
 
-static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
+static int dlpar_memory_remove_lmbs_common(u32 lmbs_to_remove, u32 drc_index)
 {
-	struct drmem_lmb *lmb;
-	int lmbs_removed = 0;
+	struct drmem_lmb *lmb, *start_lmb, *end_lmb;
 	int lmbs_available = 0;
+	int lmbs_removed = 0;
 	int rc;
-
-	pr_info("Attempting to hot-remove %d LMB(s)\n", lmbs_to_remove);
+	/*
+	 * dlpar_memory_remove_by_ic() wants to remove all
+	 * 'lmbs_to_remove' LMBs, starting from drc_index, in a
+	 * contiguous block.
+	 */
+	bool block_removal;
 
 	if (lmbs_to_remove == 0)
 		return -EINVAL;
 
+	block_removal = drc_index != 0;
+
+	if (block_removal) {
+		rc = get_lmb_range(drc_index, lmbs_to_remove, &start_lmb, &end_lmb);
+		if (rc)
+			return -EINVAL;
+	} else {
+		start_lmb = &drmem_info->lmbs[0];
+		end_lmb = &drmem_info->lmbs[drmem_info->n_lmbs];
+	}
+
 	/* Validate that there are enough LMBs to satisfy the request */
-	for_each_drmem_lmb(lmb) {
-		if (lmb_is_removable(lmb))
+	for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
+		if (lmb_is_removable(lmb)) {
 			lmbs_available++;
 
-		if (lmbs_available == lmbs_to_remove)
+			if (lmbs_available == lmbs_to_remove)
+				break;
+		} else if (block_removal) {
 			break;
+		}
 	}
 
 	if (lmbs_available < lmbs_to_remove) {
@@ -426,28 +444,40 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
 		return -EINVAL;
 	}
 
-	for_each_drmem_lmb(lmb) {
+	for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
 		rc = dlpar_remove_lmb(lmb);
-		if (rc)
-			continue;
 
-		/* Mark this lmb so we can add it later if all of the
-		 * requested LMBs cannot be removed.
-		 */
-		drmem_mark_lmb_reserved(lmb);
+		if (!rc) {
+			/* Mark this lmb so we can add it later if all of the
+			 * requested LMBs cannot be removed.
+			 */
+			drmem_mark_lmb_reserved(lmb);
 
-		lmbs_removed++;
-		if (lmbs_removed == lmbs_to_remove)
+			lmbs_removed++;
+			if (lmbs_removed == lmbs_to_remove)
+				break;
+		} else if (block_removal) {
 			break;
+		}
 	}
 
 	if (lmbs_removed != lmbs_to_remove) {
-		pr_err("Memory hot-remove failed, adding LMB's back\n");
+		if (block_removal)
+			pr_err("Memory indexed-count-remove failed, adding any removed LMBs\n");
+		else
+			pr_err("Memory hot-remove failed, adding LMB's back\n");
 
-		for_each_drmem_lmb(lmb) {
+		for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
 			if (!drmem_lmb_reserved(lmb))
 				continue;
 
+			/*
+			 * Setting the isolation state of an UNISOLATED/CONFIGURED
+			 * device to UNISOLATE is a no-op, but the hypervisor can
+			 * use it as a hint that the LMB removal failed.
+			 */
+			dlpar_unisolate_drc(lmb->drc_index);
+
 			rc = dlpar_add_lmb(lmb);
 			if (rc)
 				pr_err("Failed to add LMB back, drc index %x\n",
@@ -458,13 +488,13 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
 
 		rc = -EINVAL;
 	} else {
-		for_each_drmem_lmb(lmb) {
+		for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
 			if (!drmem_lmb_reserved(lmb))
 				continue;
 
 			dlpar_release_drc(lmb->drc_index);
-			pr_info("Memory at %llx was hot-removed\n",
-				lmb->base_addr);
+			pr_info("Memory at %llx (drc index %x) was hot-removed\n",
+				lmb->base_addr, lmb->drc_index);
 
 			drmem_remove_lmb_reservation(lmb);
 		}
@@ -474,6 +504,21 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
 	return rc;
 }
 
+static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
+{
+	pr_info("Attempting to hot-remove %d LMB(s)\n", lmbs_to_remove);
+
+	return dlpar_memory_remove_lmbs_common(lmbs_to_remove, 0);
+}
+
+static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
+{
+	pr_info("Attempting to hot-remove %u LMB(s) at %x\n",
+		lmbs_to_remove, drc_index);
+
+	return dlpar_memory_remove_lmbs_common(lmbs_to_remove, drc_index);
+}
+
 static int dlpar_memory_remove_by_index(u32 drc_index)
 {
 	struct drmem_lmb *lmb;
@@ -506,80 +551,6 @@ static int dlpar_memory_remove_by_index(u32 drc_index)
 	return rc;
 }
 
-static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
-{
-	struct drmem_lmb *lmb, *start_lmb, *end_lmb;
-	int lmbs_available = 0;
-	int rc;
-
-	pr_info("Attempting to hot-remove %u LMB(s) at %x\n",
-		lmbs_to_remove, drc_index);
-
-	if (lmbs_to_remove == 0)
-		return -EINVAL;
-
-	rc = get_lmb_range(drc_index, lmbs_to_remove, &start_lmb, &end_lmb);
-	if (rc)
-		return -EINVAL;
-
-	/* Validate that there are enough LMBs to satisfy the request */
-	for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
-		if (!lmb_is_removable(lmb))
-			break;
-
-		lmbs_available++;
-	}
-
-	if (lmbs_available < lmbs_to_remove)
-		return -EINVAL;
-
-	for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
-		rc = dlpar_remove_lmb(lmb);
-		if (rc)
-			break;
-
-		drmem_mark_lmb_reserved(lmb);
-	}
-
-	if (rc) {
-		pr_err("Memory indexed-count-remove failed, adding any removed LMBs\n");
-
-
-		for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
-			if (!drmem_lmb_reserved(lmb))
-				continue;
-
-			/*
-			 * Setting the isolation state of an UNISOLATED/CONFIGURED
-			 * device to UNISOLATE is a no-op, but the hypervisor can
-			 * use it as a hint that the LMB removal failed.
-			 */
-			dlpar_unisolate_drc(lmb->drc_index);
-
-			rc = dlpar_add_lmb(lmb);
-			if (rc)
-				pr_err("Failed to add LMB, drc index %x\n",
-				       lmb->drc_index);
-
-			drmem_remove_lmb_reservation(lmb);
-		}
-		rc = -EINVAL;
-	} else {
-		for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
-			if (!drmem_lmb_reserved(lmb))
-				continue;
-
-			dlpar_release_drc(lmb->drc_index);
-			pr_info("Memory at %llx (drc index %x) was hot-removed\n",
-				lmb->base_addr, lmb->drc_index);
-
-			drmem_remove_lmb_reservation(lmb);
-		}
-	}
-
-	return rc;
-}
-
 #else
 static inline int pseries_remove_memblock(unsigned long base,
 					  unsigned long memblock_size)
-- 
2.30.2


      parent reply	other threads:[~2021-04-30 12:11 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-30 12:09 [PATCH 0/3] Unisolate LMBs DRC on removal error + cleanups Daniel Henrique Barboza
2021-04-30 12:09 ` [PATCH 1/3] powerpc/pseries: Set UNISOLATE on dlpar_memory_remove_by_ic() error Daniel Henrique Barboza
2021-05-04  0:45   ` David Gibson
2021-04-30 12:09 ` [PATCH 2/3] hotplug-memory.c: enhance dlpar_memory_remove* LMB checks Daniel Henrique Barboza
2021-05-04  1:02   ` David Gibson
2021-05-07 16:36     ` Daniel Henrique Barboza
2021-05-09  8:43       ` David Gibson
2021-05-12 20:35     ` Daniel Henrique Barboza
2021-05-13  5:22       ` David Gibson
2021-04-30 12:09 ` Daniel Henrique Barboza [this message]

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=20210430120917.217951-4-danielhb413@gmail.com \
    --to=danielhb413@gmail.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=linuxppc-dev@lists.ozlabs.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.