All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/4] makedumpfile: optimize the way of generating incomplete kdump core
@ 2015-07-10  2:28 Zhou Wenjian
  2015-07-10  2:28 ` [PATCH v1 1/4] Add write_cd_buf Zhou Wenjian
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Zhou Wenjian @ 2015-07-10  2:28 UTC (permalink / raw)
  To: kexec


In current implementation, when generating kdump core,
it behaves as the following logic.

1. Write page header into the buffer cd_header.
2. If buffer is full, flush the buffer.
3. Write page data into the buffer cd_page.
4. If buffer is full, flush the buffer.

When enospc occurs in flushing cd_page, it always still
has much data in cd_header. The size of page data is 170
times the size of page header. It leads to that quite a
lot of page data been written into file can't be used.

The page header may also meet that. The page data can be
compressed, so the size of page data can also be much smaller
than the size of page header.

This patch set changes the logic of generating kdump core.
The new logic is:

1. Before writing page header and data into buffer, if the buffer
   will be full, write the data of the buffer into file.
2. Then, write the page header and data info buffer.

When enospc occurs in writing the cd_header into file, fill the
cd_header with zero and re-write the cd_header.
When enospc occurs in writing the cd_page into file, fill part
of the cd_header with zero according to how many pages in cd_page
have been written. 

Zhou Wenjian (4):
  Add write_cd_buf
  Add get_pfn_offset
  Add write_kdump_page
  Use write_kdump_page instead of write_cache in
    write_kdump_pages_cyclic

 makedumpfile.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 96 insertions(+), 11 deletions(-)

-- 
1.8.3.1


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v1 1/4] Add write_cd_buf
  2015-07-10  2:28 [PATCH v1 0/4] makedumpfile: optimize the way of generating incomplete kdump core Zhou Wenjian
@ 2015-07-10  2:28 ` Zhou Wenjian
  2015-07-15  2:35   ` HATAYAMA Daisuke
  2015-07-10  2:28 ` [PATCH v1 2/4] Add get_pfn_offset Zhou Wenjian
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Zhou Wenjian @ 2015-07-10  2:28 UTC (permalink / raw)
  To: kexec

write_cd_buf is used to write the data of cache_data into file.

Signed-off-by: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
---
 makedumpfile.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/makedumpfile.c b/makedumpfile.c
index cc71f20..5a53246 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -6286,6 +6286,20 @@ write_elf_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page)
 }
 
 int
+write_cd_buf(struct cache_data *cd)
+{
+	if (cd->buf_size == 0)
+		return TRUE;
+
+	if (!write_buffer(cd->fd, cd->offset, cd->buf,
+			cd->buf_size, cd->file_name)) {
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
+int
 write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page,
 			 struct page_desc *pd_zero, off_t *offset_data, struct cycle *cycle)
 {
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v1 2/4] Add get_pfn_offset
  2015-07-10  2:28 [PATCH v1 0/4] makedumpfile: optimize the way of generating incomplete kdump core Zhou Wenjian
  2015-07-10  2:28 ` [PATCH v1 1/4] Add write_cd_buf Zhou Wenjian
@ 2015-07-10  2:28 ` Zhou Wenjian
  2015-07-15  2:56   ` HATAYAMA Daisuke
  2015-07-10  2:28 ` [PATCH v1 3/4] Add write_kdump_page Zhou Wenjian
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Zhou Wenjian @ 2015-07-10  2:28 UTC (permalink / raw)
  To: kexec

get_pfn_offset is used for generating incomplete kdump core.
When enospac occurs in writing the buf cd_page, it can be used to
get how many pages have been written.

Signed-off-by: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
---
 makedumpfile.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/makedumpfile.c b/makedumpfile.c
index 5a53246..e8b52f4 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -6300,6 +6300,30 @@ write_cd_buf(struct cache_data *cd)
 }
 
 int
+get_pfn_offset(void *buf, struct cache_data *cd_page){
+	int size, file_end, offset;
+	page_desc_t *pd;
+
+	size = 0;
+	offset = 0;
+	file_end = lseek(cd_page->fd, 0, SEEK_END);
+	if (file_end < 0)
+		ERRMSG("Can't seek end of the dump file(%s).\n", cd_page->file_name);
+
+	while (TRUE) {
+		pd = (page_desc_t *)buf;
+		size += pd->size;
+		if (size > file_end - cd_page->offset)
+			break;
+
+		offset++;
+		buf += sizeof(page_desc_t);
+	}
+
+	return offset;
+}
+
+int
 write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page,
 			 struct page_desc *pd_zero, off_t *offset_data, struct cycle *cycle)
 {
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v1 3/4] Add write_kdump_page
  2015-07-10  2:28 [PATCH v1 0/4] makedumpfile: optimize the way of generating incomplete kdump core Zhou Wenjian
  2015-07-10  2:28 ` [PATCH v1 1/4] Add write_cd_buf Zhou Wenjian
  2015-07-10  2:28 ` [PATCH v1 2/4] Add get_pfn_offset Zhou Wenjian
@ 2015-07-10  2:28 ` Zhou Wenjian
  2015-07-15  3:09   ` HATAYAMA Daisuke
  2015-07-10  2:28 ` [PATCH v1 4/4] Use write_kdump_page instead of write_cache in write_kdump_pages_cyclic Zhou Wenjian
  2015-07-15  2:31 ` [PATCH v1 0/4] makedumpfile: optimize the way of generating incomplete kdump core HATAYAMA Daisuke
  4 siblings, 1 reply; 11+ messages in thread
From: Zhou Wenjian @ 2015-07-10  2:28 UTC (permalink / raw)
  To: kexec

write_kdump_page is used to write page when generating kdump core.
It mainly optimizes the way of generating incomplete core.

Signed-off-by: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
---
 makedumpfile.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/makedumpfile.c b/makedumpfile.c
index e8b52f4..74bf83e 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -6324,6 +6324,59 @@ get_pfn_offset(void *buf, struct cache_data *cd_page){
 }
 
 int
+write_kdump_page(struct cache_data *cd_header, struct cache_data *cd_page,
+		struct page_desc *pd, void *page_data)
+{
+	int written_pfns_size;
+
+	/*
+	 * if either cd_header or cd_page is nearly full,
+	 * write the buffer cd_header into dumpfile and then write the cd_page.
+	 */
+	if ( cd_header->buf_size + sizeof(pd) > cd_header->cache_size
+		|| cd_page->buf_size + pd->size > cd_page->cache_size ){
+
+		if(!write_cd_buf(cd_header)) {
+			/*
+			 * if enospc occurs in writing cd_header,
+			 * fill the cd_header with zero and re-write it
+			 * into the dumpfile.
+			 */
+			memset(cd_header->buf, 0, cd_header->cache_size);
+			write_cd_buf(cd_header);
+
+			return FALSE;
+		}
+
+		if(!write_cd_buf(cd_page)) {
+			/*
+			 * if enospc occurs in writing cd_page,
+			 * get how many pages having been written.
+			 * and fill part of cd_header with zero according to it.
+			 */
+			written_pfns_size = sizeof(page_desc_t) *
+					get_pfn_offset(cd_header->buf, cd_page);
+
+			memset(cd_header->buf, 0, cd_header->cache_size);
+			cd_header->offset += written_pfns_size;
+			cd_header->buf_size -= written_pfns_size;
+			write_cd_buf(cd_header);
+
+			return FALSE;
+		}
+		cd_header->offset += cd_header->buf_size;
+		cd_page->offset += cd_page->buf_size;
+		cd_header->buf_size = 0;
+		cd_page->buf_size = 0;
+	}
+
+	write_cache(cd_header, pd, sizeof(page_desc_t));
+	write_cache(cd_page, page_data, pd->size);
+
+	return TRUE;
+}
+
+int
 write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page,
 			 struct page_desc *pd_zero, off_t *offset_data, struct cycle *cycle)
 {
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v1 4/4] Use write_kdump_page instead of write_cache in write_kdump_pages_cyclic
  2015-07-10  2:28 [PATCH v1 0/4] makedumpfile: optimize the way of generating incomplete kdump core Zhou Wenjian
                   ` (2 preceding siblings ...)
  2015-07-10  2:28 ` [PATCH v1 3/4] Add write_kdump_page Zhou Wenjian
@ 2015-07-10  2:28 ` Zhou Wenjian
  2015-07-15  2:32   ` HATAYAMA Daisuke
  2015-07-15  2:36   ` HATAYAMA Daisuke
  2015-07-15  2:31 ` [PATCH v1 0/4] makedumpfile: optimize the way of generating incomplete kdump core HATAYAMA Daisuke
  4 siblings, 2 replies; 11+ messages in thread
From: Zhou Wenjian @ 2015-07-10  2:28 UTC (permalink / raw)
  To: kexec

Signed-off-by: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
---
 makedumpfile.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index 74bf83e..0f1aa94 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -6515,17 +6515,11 @@ write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_pag
 		pd.offset     = *offset_data;
 		*offset_data  += pd.size;
 
-                /*
-                 * Write the page header.
-                 */
-                if (!write_cache(cd_header, &pd, sizeof(page_desc_t)))
-                        goto out;
-
-                /*
-                 * Write the page data.
-                 */
-		if (!write_cache(cd_page, pd.flags ? buf_out : buf, pd.size))
-                        goto out;
+               /*
+                * Write the page header and the page data
+                */
+               if (!write_kdump_page(cd_header, cd_page, &pd, pd.flags ? buf_out : buf))
+                       goto out;
         }
 
 	ret = TRUE;
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 0/4] makedumpfile: optimize the way of generating incomplete kdump core
  2015-07-10  2:28 [PATCH v1 0/4] makedumpfile: optimize the way of generating incomplete kdump core Zhou Wenjian
                   ` (3 preceding siblings ...)
  2015-07-10  2:28 ` [PATCH v1 4/4] Use write_kdump_page instead of write_cache in write_kdump_pages_cyclic Zhou Wenjian
@ 2015-07-15  2:31 ` HATAYAMA Daisuke
  4 siblings, 0 replies; 11+ messages in thread
From: HATAYAMA Daisuke @ 2015-07-15  2:31 UTC (permalink / raw)
  To: zhouwj-fnst; +Cc: kexec

From: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
Subject: [PATCH v1 0/4] makedumpfile: optimize the way of generating incomplete kdump core
Date: Fri, 10 Jul 2015 10:28:49 +0800

I don't think this subject is good. This seems a bugfix patch set for
ENOSPC issue, not an optimization.

> 
> In current implementation, when generating kdump core,
> it behaves as the following logic.
> 
> 1. Write page header into the buffer cd_header.
> 2. If buffer is full, flush the buffer.
> 3. Write page data into the buffer cd_page.
> 4. If buffer is full, flush the buffer.
> 
> When enospc occurs in flushing cd_page, it always still
> has much data in cd_header. The size of page data is 170
> times the size of page header. It leads to that quite a
> lot of page data been written into file can't be used.
> 

Do you mean like this?

  When ENOSPC occurs in flushing cd_page, there's still data left in
  cd_header. We cannot read page data corresponding to the page
  headers in the cd_header even if the page data has been flushed into
  disk. The size of page data is 170 times larger than the size of
  page header per a single page frame. The difference becomes bigger
  if we use compression.

> The page header may also meet that. The page data can be
> compressed, so the size of page data can also be much smaller
> than the size of page header.
> 

Compression doesn't always reduce size of data. So writing this
doesn't make sense in general because.

> This patch set changes the logic of generating kdump core.
> The new logic is:

The below doesn't explain why this new logic makes sense.

> 
> 1. Before writing page header and data into buffer, if the buffer
>    will be full, write the data of the buffer into file.

                             This "the buffer" means cd_page, right?

> 2. Then, write the page header and data info buffer.

It's better to write cd_header and cd_page for clarity. "the buffer"
seems ambiguous.

> 
> When enospc occurs in writing the cd_header into file, fill the
> cd_header with zero and re-write the cd_header.
> When enospc occurs in writing the cd_page into file, fill part
> of the cd_header with zero according to how many pages in cd_page
> have been written. 
> 

> Zhou Wenjian (4):
>   Add write_cd_buf
>   Add get_pfn_offset
>   Add write_kdump_page
>   Use write_kdump_page instead of write_cache in
>     write_kdump_pages_cyclic
> 
>  makedumpfile.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 96 insertions(+), 11 deletions(-)
> 
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
--
Thanks.
HATAYAMA, Daisuke


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 4/4] Use write_kdump_page instead of write_cache in write_kdump_pages_cyclic
  2015-07-10  2:28 ` [PATCH v1 4/4] Use write_kdump_page instead of write_cache in write_kdump_pages_cyclic Zhou Wenjian
@ 2015-07-15  2:32   ` HATAYAMA Daisuke
  2015-07-15  2:36   ` HATAYAMA Daisuke
  1 sibling, 0 replies; 11+ messages in thread
From: HATAYAMA Daisuke @ 2015-07-15  2:32 UTC (permalink / raw)
  To: zhouwj-fnst; +Cc: kexec

From: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
Subject: [PATCH v1 4/4] Use write_kdump_page instead of write_cache in write_kdump_pages_cyclic
Date: Fri, 10 Jul 2015 10:28:53 +0800

Please write patch description.

> Signed-off-by: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
> ---
>  makedumpfile.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 74bf83e..0f1aa94 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -6515,17 +6515,11 @@ write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_pag
>  		pd.offset     = *offset_data;
>  		*offset_data  += pd.size;
>  
> -                /*
> -                 * Write the page header.
> -                 */
> -                if (!write_cache(cd_header, &pd, sizeof(page_desc_t)))
> -                        goto out;
> -
> -                /*
> -                 * Write the page data.
> -                 */
> -		if (!write_cache(cd_page, pd.flags ? buf_out : buf, pd.size))
> -                        goto out;
> +               /*
> +                * Write the page header and the page data
> +                */
> +               if (!write_kdump_page(cd_header, cd_page, &pd, pd.flags ? buf_out : buf))
> +                       goto out;
>          }
>  
>  	ret = TRUE;
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
--
Thanks.
HATAYAMA, Daisuke


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/4] Add write_cd_buf
  2015-07-10  2:28 ` [PATCH v1 1/4] Add write_cd_buf Zhou Wenjian
@ 2015-07-15  2:35   ` HATAYAMA Daisuke
  0 siblings, 0 replies; 11+ messages in thread
From: HATAYAMA Daisuke @ 2015-07-15  2:35 UTC (permalink / raw)
  To: zhouwj-fnst; +Cc: kexec

From: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
Subject: [PATCH v1 1/4] Add write_cd_buf
Date: Fri, 10 Jul 2015 10:28:50 +0800

> write_cd_buf is used to write the data of cache_data into file.
> 

I think this patch should be merged into 3/4.

> Signed-off-by: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
> ---
>  makedumpfile.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index cc71f20..5a53246 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -6286,6 +6286,20 @@ write_elf_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page)
>  }
>  
>  int
> +write_cd_buf(struct cache_data *cd)
> +{
> +	if (cd->buf_size == 0)
> +		return TRUE;
> +
> +	if (!write_buffer(cd->fd, cd->offset, cd->buf,
> +			cd->buf_size, cd->file_name)) {
> +		return FALSE;
> +	}
> +
> +	return TRUE;
> +}
> +
> +int
>  write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page,
>  			 struct page_desc *pd_zero, off_t *offset_data, struct cycle *cycle)
>  {
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
--
Thanks.
HATAYAMA, Daisuke


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 4/4] Use write_kdump_page instead of write_cache in write_kdump_pages_cyclic
  2015-07-10  2:28 ` [PATCH v1 4/4] Use write_kdump_page instead of write_cache in write_kdump_pages_cyclic Zhou Wenjian
  2015-07-15  2:32   ` HATAYAMA Daisuke
@ 2015-07-15  2:36   ` HATAYAMA Daisuke
  1 sibling, 0 replies; 11+ messages in thread
From: HATAYAMA Daisuke @ 2015-07-15  2:36 UTC (permalink / raw)
  To: zhouwj-fnst; +Cc: kexec

From: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
Subject: [PATCH v1 4/4] Use write_kdump_page instead of write_cache in write_kdump_pages_cyclic
Date: Fri, 10 Jul 2015 10:28:53 +0800

I think this patch should be merged into 3/4.

> Signed-off-by: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
> ---
>  makedumpfile.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 74bf83e..0f1aa94 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -6515,17 +6515,11 @@ write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_pag
>  		pd.offset     = *offset_data;
>  		*offset_data  += pd.size;
>  
> -                /*
> -                 * Write the page header.
> -                 */
> -                if (!write_cache(cd_header, &pd, sizeof(page_desc_t)))
> -                        goto out;
> -
> -                /*
> -                 * Write the page data.
> -                 */
> -		if (!write_cache(cd_page, pd.flags ? buf_out : buf, pd.size))
> -                        goto out;
> +               /*
> +                * Write the page header and the page data
> +                */
> +               if (!write_kdump_page(cd_header, cd_page, &pd, pd.flags ? buf_out : buf))
> +                       goto out;
>          }
>  
>  	ret = TRUE;
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 2/4] Add get_pfn_offset
  2015-07-10  2:28 ` [PATCH v1 2/4] Add get_pfn_offset Zhou Wenjian
@ 2015-07-15  2:56   ` HATAYAMA Daisuke
  0 siblings, 0 replies; 11+ messages in thread
From: HATAYAMA Daisuke @ 2015-07-15  2:56 UTC (permalink / raw)
  To: zhouwj-fnst; +Cc: kexec

From: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
Subject: [PATCH v1 2/4] Add get_pfn_offset
Date: Fri, 10 Jul 2015 10:28:51 +0800

> get_pfn_offset is used for generating incomplete kdump core.
> When enospac occurs in writing the buf cd_page, it can be used to
> get how many pages have been written.
> 

This patch should be merged into 3/4. This patch description is an
expalnation of get_pfn_offset(). This should be just above
get_pfn_offset() as a comment.

> Signed-off-by: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
> ---
>  makedumpfile.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 5a53246..e8b52f4 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -6300,6 +6300,30 @@ write_cd_buf(struct cache_data *cd)
>  }
>  
>  int
> +get_pfn_offset(void *buf, struct cache_data *cd_page){
> +	int size, file_end, offset;
> +	page_desc_t *pd;
> +
> +	size = 0;
> +	offset = 0;

Put these initialization just before the while loop below.

Is this really offset? This looks to me the number of pages.

> +	file_end = lseek(cd_page->fd, 0, SEEK_END);
> +	if (file_end < 0)
> +		ERRMSG("Can't seek end of the dump file(%s).\n", cd_page->file_name);
> +
> +	while (TRUE) {
> +		pd = (page_desc_t *)buf;
> +		size += pd->size;
> +		if (size > file_end - cd_page->offset)
> +			break;
> +
> +		offset++;
> +		buf += sizeof(page_desc_t);
> +	}

How about like this? I use nr_pages not offset.

page_desc_t *pd = buf;

...<skip>...

size = pd->size;
nr_pages = 0;
while (size <= file_end - cd_page->offset) {
    nr_pages++;
    pd++;
    size += pd->size;
}

Please rename get_pfn_offset(), too.

> +
> +	return offset;
> +}
> +
> +int
>  write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page,
>  			 struct page_desc *pd_zero, off_t *offset_data, struct cycle *cycle)
>  {
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 3/4] Add write_kdump_page
  2015-07-10  2:28 ` [PATCH v1 3/4] Add write_kdump_page Zhou Wenjian
@ 2015-07-15  3:09   ` HATAYAMA Daisuke
  0 siblings, 0 replies; 11+ messages in thread
From: HATAYAMA Daisuke @ 2015-07-15  3:09 UTC (permalink / raw)
  To: zhouwj-fnst; +Cc: kexec

From: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
Subject: [PATCH v1 3/4] Add write_kdump_page
Date: Fri, 10 Jul 2015 10:28:52 +0800

> write_kdump_page is used to write page when generating kdump core.
> It mainly optimizes the way of generating incomplete core.
> 
> Signed-off-by: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
> ---
>  makedumpfile.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index e8b52f4..74bf83e 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -6324,6 +6324,59 @@ get_pfn_offset(void *buf, struct cache_data *cd_page){
>  }
>  
>  int
> +write_kdump_page(struct cache_data *cd_header, struct cache_data *cd_page,
> +		struct page_desc *pd, void *page_data)
> +{
> +	int written_pfns_size;
> +
> +	/*
> +	 * if either cd_header or cd_page is nearly full,
> +	 * write the buffer cd_header into dumpfile and then write the cd_page.
> +	 */

Please begin comments by capital letter. They should be formal.

Also, please explain why. What currently this comment says is obvious
from the below.

> +	if ( cd_header->buf_size + sizeof(pd) > cd_header->cache_size

No space needed here.

Also, sizeof(pd) should have been sizeof(*pd), right?

if (cd_header->buf_size + sizeof(*pd) > cd_header->cache_size
 		|| cd_page->buf_size + pd->size > cd_page->cache_size){

> +		|| cd_page->buf_size + pd->size > cd_page->cache_size ){
> +
> +		if(!write_cd_buf(cd_header)) {

Space is needed here.

      	 	if (!write_cd_buf(cd_header)) {

> +			/*
> +			 * if enospc occurs in writing cd_header,
> +			 * fill the cd_header with zero and re-write it
> +			 * into the dumpfile.
> +			 */

Also, this comment is meangless, no additional information...

> +			memset(cd_header->buf, 0, cd_header->cache_size);
> +			write_cd_buf(cd_header);
> +
> +			return FALSE;
> +		}
> +
> +		if(!write_cd_buf(cd_page)) {
> +			/*
> +			 * if enospc occurs in writing cd_page,
> +			 * get how many pages having been written.
> +			 * and fill part of cd_header with zero according to it.
> +			 */
> +			written_pfns_size = sizeof(page_desc_t) *
> +					get_pfn_offset(cd_header->buf, cd_page);

Hmm, so get_pfn_offset() returns the number of pages? If so, this
function name of get_pfn_offset() is very confusing.

Is this variable name written_pfns_size good? That is, I associate
page frame with pfn, but this variable should represent size of page
headers.

> +
> +			memset(cd_header->buf, 0, cd_header->cache_size);
> +			cd_header->offset += written_pfns_size;
> +			cd_header->buf_size -= written_pfns_size;
> +			write_cd_buf(cd_header);
> +
> +			return FALSE;
> +		}
> +		cd_header->offset += cd_header->buf_size;
> +		cd_page->offset += cd_page->buf_size;
> +		cd_header->buf_size = 0;
> +		cd_page->buf_size = 0;
> +	}
> +
> +	write_cache(cd_header, pd, sizeof(page_desc_t));
> +	write_cache(cd_page, page_data, pd->size);
> +
> +	return TRUE;
> +}
> +
> +int
>  write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page,
>  			 struct page_desc *pd_zero, off_t *offset_data, struct cycle *cycle)
>  {
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
--
Thanks.
HATAYAMA, Daisuke


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-07-15  3:10 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-10  2:28 [PATCH v1 0/4] makedumpfile: optimize the way of generating incomplete kdump core Zhou Wenjian
2015-07-10  2:28 ` [PATCH v1 1/4] Add write_cd_buf Zhou Wenjian
2015-07-15  2:35   ` HATAYAMA Daisuke
2015-07-10  2:28 ` [PATCH v1 2/4] Add get_pfn_offset Zhou Wenjian
2015-07-15  2:56   ` HATAYAMA Daisuke
2015-07-10  2:28 ` [PATCH v1 3/4] Add write_kdump_page Zhou Wenjian
2015-07-15  3:09   ` HATAYAMA Daisuke
2015-07-10  2:28 ` [PATCH v1 4/4] Use write_kdump_page instead of write_cache in write_kdump_pages_cyclic Zhou Wenjian
2015-07-15  2:32   ` HATAYAMA Daisuke
2015-07-15  2:36   ` HATAYAMA Daisuke
2015-07-15  2:31 ` [PATCH v1 0/4] makedumpfile: optimize the way of generating incomplete kdump core HATAYAMA Daisuke

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.