From: Zhou Wenjian <zhouwj-fnst@xxxxxxxxxxxxxx>
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 at 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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
--
Thanks.
HATAYAMA, Daisuke
