All of lore.kernel.org
 help / color / mirror / Atom feed
From: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
To: Baoquan He <bhe@redhat.com>
Cc: kumagai-atsushi@mxc.nes.nec.co.jp, kexec@lists.infradead.org
Subject: Re: [v2 2/3] makedumpfile: use struct cycle to update cyclic region and clean up
Date: Mon, 27 Jan 2014 13:03:02 +0900	[thread overview]
Message-ID: <52E5DA76.7070702@jp.fujitsu.com> (raw)
In-Reply-To: <1390470454-14272-3-git-send-email-bhe@redhat.com>

(2014/01/23 18:47), Baoquan He wrote:

> @@ -3301,18 +3301,15 @@ set_bitmap(struct dump_bitmap *bitmap, unsigned long long pfn,
>   }
>   
>   int
> -set_bitmap_cyclic(char *bitmap, unsigned long long pfn, int val)
> +set_bitmap_cyclic(char *bitmap, unsigned long long pfn, int val, struct cycle *cycle)
>   {
>   	int byte, bit;
>   
> -	if (pfn < info->cyclic_start_pfn || info->cyclic_end_pfn <= pfn)
> -		return FALSE;
> -

Removing this check changes behaviour, not just clean up, so should be separated into another patch.

Further, this change seems buggy because, in addition to Kumaga-san's comment, memory outside of
cycle region can be passed to here. Please look at the below.

> @@ -4782,10 +4780,10 @@ exclude_unnecessary_pages_cyclic(void)
>   			if (mmd->mem_map == NOT_MEMMAP_ADDR)
>   				continue;
>   
> -			if (mmd->pfn_end >= info->cyclic_start_pfn &&
> -			    mmd->pfn_start <= info->cyclic_end_pfn) {
> +			if (mmd->pfn_end >= cycle->start_pfn &&
> +			    mmd->pfn_start <= cycle->end_pfn) {
>   				if (!__exclude_unnecessary_pages(mmd->mem_map,
> -								 mmd->pfn_start, mmd->pfn_end))
> +								 mmd->pfn_start, mmd->pfn_end, cycle))
>   					return FALSE;
>   			}
>   		}

After this clean up, __exclude_unnecessary_pages() processes memory within cycle region only.
So, it's necessary to pass adjusted range, like:

if (MAX(mmd->pfn_start, cycle->start_pfn) < MIN(mmd->pfn_end, cycle->end_pfn))
  __exclude_unnecessary_pages(mmd->mem_map, MAX(mmd->pfn_start, cycle->start_pfn), MIN(mmd->pfn_end, cycle->end_pfn))

-- 
Thanks.
HATAYAMA, Daisuke


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

  reply	other threads:[~2014-01-27  4:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-23  9:47 [v2 0/3] Introduce struct cycle to store cyclic region and clean up Baoquan He
2014-01-23  9:47 ` [v2 1/3] makedumpfile: introduce struct cycle to store the cyclic region Baoquan He
2014-01-23  9:47 ` [v2 2/3] makedumpfile: use struct cycle to update cyclic region and clean up Baoquan He
2014-01-27  4:03   ` HATAYAMA Daisuke [this message]
2014-01-27  8:55     ` Baoquan He
2014-01-28  7:49       ` Atsushi Kumagai
2014-02-08  9:09         ` Baoquan He
2014-01-23  9:47 ` [v2 3/3] makedumpfile: remove member variables representing cyclic pfn region in struct DumpInfo Baoquan He

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=52E5DA76.7070702@jp.fujitsu.com \
    --to=d.hatayama@jp.fujitsu.com \
    --cc=bhe@redhat.com \
    --cc=kexec@lists.infradead.org \
    --cc=kumagai-atsushi@mxc.nes.nec.co.jp \
    /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.