comments below
On 01/17/14 08:46, qiaonuohan wrote:
> add some members to DumpState that will be used in writing vmcore in
> kdump-compressed format. some of them, like page_size, will be initialized
> in the patch.
>
> Signed-off-by: Qiao Nuohan <qiaonuo...@cn.fujitsu.com>
> ---
> dump.c | 30 ++++++++++++++++++++++++++++++
> include/sysemu/dump.h | 7 +++++++
> 2 files changed, 37 insertions(+), 0 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index 2b940bd..bf7d31d 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -79,6 +79,16 @@ typedef struct DumpState {
>
> uint8_t *note_buf; /* buffer for notes */
> size_t note_buf_offset; /* the writing place in note_buf */
> + uint32_t nr_cpus; /* number of guest's cpu */
> + size_t page_size; /* guest's page size */
> + uint32_t page_shift; /* guest's page shift */
> + uint64_t max_mapnr; /* the biggest guest's phys-mem's number */
> + size_t len_dump_bitmap; /* the size of the place used to store
> + dump_bitmap in vmcore */
> + off_t offset_dump_bitmap; /* offset of dump_bitmap part in vmcore */
> + off_t offset_page; /* offset of page part in vmcore */
> + size_t num_dumpable; /* number of page that can be dumped */
> + uint32_t flag_compress; /* indicate the compression format */
> } DumpState;
v6 06/11 addded these, but we have the following changes here:
- flag_flatten is gone, OK,
- bunch of comments, good,
- page_shift and num_dumpable are now added at once (originally in v6
07/11).
>
> static int dump_cleanup(DumpState *s)
> @@ -796,6 +806,16 @@ static ram_addr_t get_start_block(DumpState *s)
> return -1;
> }
>
> +static void get_max_mapnr(DumpState *s)
> +{
> + MemoryMapping *memory_mapping;
> +
> + QTAILQ_FOREACH(memory_mapping, &s->list.head, next) {
> + s->max_mapnr = paddr_to_pfn(memory_mapping->phys_addr +
> + memory_mapping->length, s->page_shift);
> + }
> +}
> +
> static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
> int64_t begin, int64_t length, Error **errp)
> {
This is from v6 10/11, OK.
> @@ -864,6 +884,16 @@ static int dump_init(DumpState *s, int fd, bool paging,
> bool has_filter,
> qemu_get_guest_simple_memory_mapping(&s->list,
> &s->guest_phys_blocks);
> }
>
> + s->nr_cpus = nr_cpus;
> + s->page_size = TARGET_PAGE_SIZE;
> + s->page_shift = ffs(s->page_size) - 1;
> +
> + get_max_mapnr(s);
Again from v6 10/11, good. The flag_flatten assignment has been dropped.
Initialization seems to happen in a good spot this time too.
> +
> + uint64_t tmp;
> + tmp = DIV_ROUND_UP(DIV_ROUND_UP(s->max_mapnr, CHAR_BIT), s->page_size);
> + s->len_dump_bitmap = tmp * s->page_size;
> +
> if (s->has_filter) {
> memory_mapping_filter(&s->list, s->begin, s->length);
> }
Again from v6 10/11.
These assignments now all occur without depending on a user request for
a compressed dump (kept this way in v7 12/13 too), but they are not
costly. The loop in get_max_mapnr() iterates over less than 10 mappings
in the non-paging dump case, and in the paging dump case it also
shouldn't be more than a hundred or so (as I recall from earlier
testing). This might be worth some regression-testing (perf-wise), but
it looks OK to me.
> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> index b32b390..995bf47 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -20,6 +20,13 @@
> #define VERSION_FLAT_HEADER (1) /* version of flattened format */
> #define END_FLAG_FLAT_HEADER (-1)
>
> +#define ARCH_PFN_OFFSET (0)
> +
> +#define paddr_to_pfn(X, page_shift) \
> + (((unsigned long long)(X) >> (page_shift)) - ARCH_PFN_OFFSET)
>From v6 07/11, needed by get_max_mapnr().
> +#define pfn_to_paddr(X, page_shift) \
> + (((unsigned long long)(X) + ARCH_PFN_OFFSET) << (page_shift))
> +
> typedef struct ArchDumpInfo {
> int d_machine; /* Architecture */
> int d_endian; /* ELFDATA2LSB or ELFDATA2MSB */
>
>From v6 09/11. Not strictly needed right now, but it does make sense for
consistency.
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
