All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
To: Simon Horman <horms@verge.net.au>
Cc: "kexec@lists.infradead.org" <kexec@lists.infradead.org>
Subject: [PATCH] kexec,x86: code optimization and adjustment for add_memmap and delete_memmap
Date: Mon, 25 Feb 2013 20:38:41 +0800	[thread overview]
Message-ID: <512B5B51.2060209@cn.fujitsu.com> (raw)

The code in the two functions seems a little messy, So I modify
some of them, trying to make the logic more clearly.

For example,
code before:

         for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
                 mstart = memmap_p[i].start;
                 mend = memmap_p[i].end;
                 if (mstart == 0 && mend == 0)
                         break;

for we already have nr_entries for this memmap_p array, so we needn't
have a check in every loop to see if we have go through the whole array.
code after:

         for (i = 0; i < nr_entries; i++) {
 		mstart = memmap_p[i].start;
 		mend = memmap_p[i].end;

Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
 kexec/arch/i386/crashdump-x86.c |   93 ++++++++++++++++++---------------------
 1 files changed, 43 insertions(+), 50 deletions(-)

diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
index 245402c..63f6b2b 100644
--- a/kexec/arch/i386/crashdump-x86.c
+++ b/kexec/arch/i386/crashdump-x86.c
@@ -518,21 +518,22 @@ static int exclude_region(int *nr_ranges, uint64_t start, uint64_t end)
 
 /* Adds a segment from list of memory regions which new kernel can use to
  * boot. Segment start and end should be aligned to 1K boundary. */
-static int add_memmap(struct memory_range *memmap_p, unsigned long long addr,
-								size_t size)
+static int add_memmap(struct memory_range *memmap_p,
+		      unsigned long long addr,
+		      size_t size)
 {
 	int i, j, nr_entries = 0, tidx = 0, align = 1024;
 	unsigned long long mstart, mend;
 
 	/* Do alignment check. */
-	if ((addr%align) || (size%align))
+	if ((addr % align) || (size % align))
 		return -1;
 
 	/* Make sure at least one entry in list is free. */
-	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
+	for (i = 0; i < CRASH_MAX_MEMMAP_NR; i++) {
 		mstart = memmap_p[i].start;
 		mend = memmap_p[i].end;
-		if (!mstart  && !mend)
+		if (!mstart && !mend)
 			break;
 		else
 			nr_entries++;
@@ -540,31 +541,29 @@ static int add_memmap(struct memory_range *memmap_p, unsigned long long addr,
 	if (nr_entries == CRASH_MAX_MEMMAP_NR)
 		return -1;
 
-	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
+	for (i = 0; i < nr_entries; i++) {
 		mstart = memmap_p[i].start;
 		mend = memmap_p[i].end;
-		if (mstart == 0 && mend == 0)
-			break;
-		if (mstart <= (addr+size-1) && mend >=addr)
+
+		if (mstart <= (addr + size - 1) && mend >= addr)
 			/* Overlapping region. */
 			return -1;
 		else if (addr > mend)
-			tidx = i+1;
+			tidx = i + 1;
 	}
-		/* Insert the memory region. */
-		for (j = nr_entries-1; j >= tidx; j--)
-			memmap_p[j+1] = memmap_p[j];
-		memmap_p[tidx].start = addr;
-		memmap_p[tidx].end = addr + size - 1;
+
+	/* Insert the memory region. */
+	for (j = nr_entries - 1; j >= tidx; j--)
+		memmap_p[j+1] = memmap_p[j];
+	memmap_p[tidx].start = addr;
+	memmap_p[tidx].end = addr + size - 1;
+	nr_entries++;
 
 	dbgprintf("Memmap after adding segment\n");
-	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
+	for (i = 0; i < nr_entries; i++) {
 		mstart = memmap_p[i].start;
 		mend = memmap_p[i].end;
-		if (mstart == 0 && mend == 0)
-			break;
-		dbgprintf("%016llx - %016llx\n",
-			mstart, mend);
+		dbgprintf("%016llx - %016llx\n", mstart, mend);
 	}
 
 	return 0;
@@ -572,19 +571,20 @@ static int add_memmap(struct memory_range *memmap_p, unsigned long long addr,
 
 /* Removes a segment from list of memory regions which new kernel can use to
  * boot. Segment start and end should be aligned to 1K boundary. */
-static int delete_memmap(struct memory_range *memmap_p, unsigned long long addr,
-								size_t size)
+static int delete_memmap(struct memory_range *memmap_p,
+			 unsigned long long addr,
+			 size_t size)
 {
 	int i, j, nr_entries = 0, tidx = -1, operation = 0, align = 1024;
 	unsigned long long mstart, mend;
 	struct memory_range temp_region;
 
 	/* Do alignment check. */
-	if ((addr%align) || (size%align))
+	if ((addr % align) || (size % align))
 		return -1;
 
 	/* Make sure at least one entry in list is free. */
-	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
+	for (i = 0; i < CRASH_MAX_MEMMAP_NR; i++) {
 		mstart = memmap_p[i].start;
 		mend = memmap_p[i].end;
 		if (!mstart  && !mend)
@@ -596,20 +596,16 @@ static int delete_memmap(struct memory_range *memmap_p, unsigned long long addr,
 		/* List if full */
 		return -1;
 
-	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
+	for (i = 0; i < nr_entries; i++) {
 		mstart = memmap_p[i].start;
 		mend = memmap_p[i].end;
-		if (mstart == 0 && mend == 0)
-			/* Did not find the segment in the list. */
-			return -1;
+
 		if (mstart <= addr && mend >= (addr + size - 1)) {
 			if (mstart == addr && mend == (addr + size - 1)) {
 				/* Exact match. Delete region */
 				operation = -1;
 				tidx = i;
-				break;
-			}
-			if (mstart != addr && mend != (addr + size - 1)) {
+			} else if (mstart != addr && mend != (addr + size - 1)) {
 				/* Split in two */
 				memmap_p[i].end = addr - 1;
 				temp_region.start = addr + size;
@@ -617,41 +613,38 @@ static int delete_memmap(struct memory_range *memmap_p, unsigned long long addr,
 				temp_region.type = memmap_p[i].type;
 				operation = 1;
 				tidx = i;
-				break;
-			}
-
-			/* No addition/deletion required. Adjust the existing.*/
-			if (mstart != addr) {
-				memmap_p[i].end = addr - 1;
-				break;
 			} else {
-				memmap_p[i].start = addr + size;
-				break;
+				/* No addition/deletion required. Adjust the existing.*/
+				if (mstart != addr)
+					memmap_p[i].end = addr - 1;
+				else
+					memmap_p[i].start = addr + size;
 			}
+			break;
 		}
 	}
-	if ((operation == 1) && tidx >=0) {
+
+	if (operation == 1 && tidx >= 0) {
 		/* Insert the split memory region. */
-		for (j = nr_entries-1; j > tidx; j--)
+		for (j = nr_entries - 1; j > tidx; j--)
 			memmap_p[j+1] = memmap_p[j];
 		memmap_p[tidx+1] = temp_region;
+		nr_entries++;
 	}
-	if ((operation == -1) && tidx >=0) {
+
+	if (operation == -1 && tidx >= 0) {
 		/* Delete the exact match memory region. */
-		for (j = i+1; j < CRASH_MAX_MEMMAP_NR; j++)
+		for (j = i + 1; j < nr_entries; j++)
 			memmap_p[j-1] = memmap_p[j];
 		memmap_p[j-1].start = memmap_p[j-1].end = 0;
+		nr_entries--;
 	}
 
 	dbgprintf("Memmap after deleting segment\n");
-	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
+	for (i = 0; i < nr_entries; i++) {
 		mstart = memmap_p[i].start;
 		mend = memmap_p[i].end;
-		if (mstart == 0 && mend == 0) {
-			break;
-		}
-		dbgprintf("%016llx - %016llx\n",
-			mstart, mend);
+		dbgprintf("%016llx - %016llx\n", mstart, mend);
 	}
 
 	return 0;
-- 
1.7.1

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

             reply	other threads:[~2013-02-25 12:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-25 12:38 Zhang Yanfei [this message]
2013-02-26  0:57 ` [PATCH] kexec,x86: code optimization and adjustment for add_memmap and delete_memmap HATAYAMA Daisuke
2013-02-26  0:58 ` HATAYAMA Daisuke
2013-03-05  2:25   ` Simon Horman
  -- strict thread matches above, loose matches on Subject: below --
2012-12-25  8:31 Zhang Yanfei

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=512B5B51.2060209@cn.fujitsu.com \
    --to=zhangyanfei@cn.fujitsu.com \
    --cc=horms@verge.net.au \
    --cc=kexec@lists.infradead.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.