On Fri, Aug 05, 2016 at 11:46:33AM +0200, Laurent Vivier wrote: > > > On 05/08/2016 11:26, Andrew Jones wrote: > > On Thu, Aug 04, 2016 at 10:41:16AM +0200, Laurent Vivier wrote: > >> > >> > >> On 04/08/2016 04:38, David Gibson wrote: > >>> On Wed, Aug 03, 2016 at 09:55:07PM +0200, Laurent Vivier wrote: > >>>> If we don't provide the page size in target-ppc:cpu_get_dump_info(), > >>>> the default one (TARGET_PAGE_SIZE, 4KB) is used to create > >>>> the compressed dump. It works fine with Macintosh, but not with > >>>> pseries as the kernel default page size is 64KB. > >>>> > >>>> Without this patch, if we generate a compressed dump in the QEMU monitor: > >>>> > >>>> (qemu) dump-guest-memory -z qemu.dump > >>>> > >>>> This dump cannot be read by crash: > >>>> > >>>> # crash vmlinux qemu.dump > >>>> ... > >>>> WARNING: cannot translate vmemmap kernel virtual addresses: > >>>> commands requiring page structure contents will fail > >>>> ... > >>>> > >>>> Signed-off-by: Laurent Vivier <lviv...@redhat.com> > >>>> --- > >>>> target-ppc/arch_dump.c | 5 +++++ > >>>> 1 file changed, 5 insertions(+) > >>> > >>> Urgh.. so, really the page size used by the guest kernel is a > >>> guest-side detail, and it's certainly possible to build a 4kiB page > >>> guest kernel, although 64kiB is the norm. > >> > >> virtio-balloon doesn't work with 4K kernel. > >> > >>> This might be the best we can do, but it'd be nice if we could probe > >>> or otherwise avoid relying on this assumption about the guest kernel. > >> > >> I agree with you but none of the other architectures probes for the page > >> size. > >> > >> For instance ARM: |I cc: Drew to know how he has chosen the values] > >> > >> if (arm_feature(env, ARM_FEATURE_AARCH64)) { > >> ... > >> info->page_size = (1 << 16); > >> ... > >> } else { > >> ... > >> info->page_size = (1 << 12); > >> ... > >> } > >> > > > > info->page_size is used to determine the dumpfile's block size. The > > block size needs to be at least the page size, but a multiple of page > > size works fine too. As we can't probe for the currently used guest > > page size, and a multiple of page size is fine, then using the guest's > > maximum allowed page size is the best we can do. > > Thank you for the explanation. > > So we can unconditionally use 64KB, even for mac99 with a 64bit > processor or a 32bit processor (that are always 4K page size)?
If they're always going to be 4K, then I'd leave them 4K. This is because I don't know enough about how the crash utility works today, or will work in the future, to know what the trade-offs are. IOW, if we assume that block-size == page-size is optimal, then we want that, or only to diverge minimally from it when necessary. That said, from what I do know about dumpfiles and the crash utility, I can't see how it would matter much, except for wasting more space with the header block (the header block isn't compressed) > > The maximum page size in the kernel can be 256kB [1], should we use this > value instead? I like how you use machine type to decide. If you have machine types that may use 256K, then you can add new conditions for them, but, for the reason I stated above, I wouldn't unconditionally use a block size that's much larger than necessary. Thanks, drew > > Laurent > > [1] linux/arch/powerpc/include/asm/page.h > /* > * On regular PPC32 page size is 4K (but we support 4K/16K/64K/256K pages > * on PPC44x). For PPC64 we support either 4K or 64K software > * page size. When using 64K pages however, whether we are really supporting > * 64K pages in HW or not is irrelevant to those definitions. > */ > #if defined(CONFIG_PPC_256K_PAGES) > #define PAGE_SHIFT 18 > #elif defined(CONFIG_PPC_64K_PAGES) > #define PAGE_SHIFT 16 > #elif defined(CONFIG_PPC_16K_PAGES) > #define PAGE_SHIFT 14 > #else > #define PAGE_SHIFT 12 > #endif > >