lkml.org 
[lkml]   [2015]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] PM / Hiberante : optimize swsusp_free()
Date
On Wednesday, March 25, 2015 01:49:36 AM Yeon, JeHyeon wrote:
> From 6cb5fffc41911a29212be52d4ce7e481f5077ccf Mon Sep 17 00:00:00 2001
> From: "Tom(JeHyeon) Yeon" <tom.yeon@windriver.com>
> Date: Thu, 19 Mar 2015 17:10:45 +0900
> Subject: [PATCH] PM / Hiberante : optimize swsusp_free()
>
> Our team developed the snapshot booting.
> Fisrt of all, make a snapshot image, compress it and finally save it
> in the storage(like mmc).
> When the system is booting next time, bootloader read it from mmc,
> decompress it and jump to the kernel.
> In this circumstance, mili seconds is very important.
> So, I prepared this patch, but not applied because I missed the time
> to apply it.
>
> And, I came across to find commit fdd64ed.
> It's very similar to the patch I prepared.

So the part of the changelog above this line is not really relevant.

But the below is OK.

> I think do { ... } while (fb_pfn != fr_pfn) operation is very similar
> to my patch. but, it takes a little more time to iterate.
> So suggest to iterate one of two maps and check whether the other map
> has the same pfn, finally free the page.
>
> Signed-off-by: Tom(JeHyeon) Yeon <tom.yeon@windriver.com>

As for the patch itself ->

> ---
> kernel/power/snapshot.c | 43 ++++++++++---------------------------------
> 1 file changed, 10 insertions(+), 33 deletions(-)
>
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index c24d5a2..a1ad801 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -726,14 +726,6 @@ static void memory_bm_clear_bit(struct memory_bitmap *bm, unsigned long pfn)
> clear_bit(bit, addr);
> }
>
> -static void memory_bm_clear_current(struct memory_bitmap *bm)
> -{
> - int bit;
> -
> - bit = max(bm->cur.node_bit - 1, 0);
> - clear_bit(bit, bm->cur.node->data);
> -}
> -
> static int memory_bm_test_bit(struct memory_bitmap *bm, unsigned long pfn)
> {
> void *addr;
> @@ -1342,36 +1334,21 @@ static struct memory_bitmap copy_bm;
>
> void swsusp_free(void)
> {
> - unsigned long fb_pfn, fr_pfn;
> + unsigned long pfn;
>
> if (!forbidden_pages_map || !free_pages_map)
> goto out;
>
> memory_bm_position_reset(forbidden_pages_map);
> - memory_bm_position_reset(free_pages_map);
> -
> -loop:
> - fr_pfn = memory_bm_next_pfn(free_pages_map);
> - fb_pfn = memory_bm_next_pfn(forbidden_pages_map);
> -
> - /*
> - * Find the next bit set in both bitmaps. This is guaranteed to
> - * terminate when fb_pfn == fr_pfn == BM_END_OF_MAP.
> - */
> - do {
> - if (fb_pfn < fr_pfn)
> - fb_pfn = memory_bm_next_pfn(forbidden_pages_map);
> - if (fr_pfn < fb_pfn)
> - fr_pfn = memory_bm_next_pfn(free_pages_map);
> - } while (fb_pfn != fr_pfn);
> -
> - if (fr_pfn != BM_END_OF_MAP && pfn_valid(fr_pfn)) {
> - struct page *page = pfn_to_page(fr_pfn);
> -
> - memory_bm_clear_current(forbidden_pages_map);
> - memory_bm_clear_current(free_pages_map);
> - __free_page(page);
> - goto loop;
> + for ( ; ; ) {
> + pfn = memory_bm_next_pfn(forbidden_pages_map);
> + if (BM_END_OF_MAP == pfn)

-> First, the usual way of writing such things is

if (pfn == BM_END_OF_MAP)

(ie. the variable on the left-hand side of the operator).

Second, don't you need to do the pfn_valid() check here too?

> + break;
> + if (memory_bm_test_bit(free_pages_map, pfn)) {
> + memory_bm_clear_bit(forbidden_pages_map, pfn);
> + memory_bm_clear_bit(free_pages_map, pfn);
> + __free_page(pfn_to_page(pfn));
> + }
> }
>
> out:


--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.


\
 
 \ /
  Last update: 2015-04-11 02:21    [W:0.080 / U:0.836 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site