On 10/15/2013 04:22 PM, HATAYAMA Daisuke wrote:
(2013/10/15 16:55), Jingbai Ma wrote:
On 10/15/2013 01:58 PM, HATAYAMA Daisuke wrote:
(2013/10/14 21:16), Jingbai Ma wrote:
<cut>
@@ -125,7 +126,7 @@ get_max_mapnr(void)
unsigned long long max_paddr;
if (info->flag_refiltering) {
- info->max_mapnr = info->dh_memory->max_mapnr;
+ info->max_mapnr = info->kh_memory->max_mapnr_64;
return TRUE;
}
Please:
if (dh.header_version < 6)
info->max_mapnr = info->dh_memmory->max_mapnr;
else
info->max_mapnr = info->kh_memory->max_mapnr_64;
If we deal the max_mapnr_64 as below I did, we do not have to check
header_version everywhere when we need to the value max_mapnr. I just
set it to max_mapnr_64 regardless it's old version or new in the first
place we get it. It can simplify the code logic in all code. Or we
have to add this version check, it's also error prone.
No, it complicates code logic in the sense that header on the memory and
header on the disk are not identical.
It seems to me that the case where checking header version is definitely
necessary is only when assigning either of max_mapnr values to
info->max_mapnr,
after which, it's enough to refer to info->max_mapnr.
OK, got it. I will send a new patch soon.
Thanks!
@@ -783,6 +784,10 @@ get_kdump_compressed_header_info(char *filename)
ERRMSG("header does not have dump_level member\n");
return FALSE;
}
+
+ if (dh.header_version < 6)
+ kh.max_mapnr_64 = dh.max_mapnr;
+
Again, please don't do this. It's confusing if in-memory header data
is not identical to in-disk one.
I have explained the reason why I set the kh.max_mapnr_64 for old
version here.
If you still think we shouldn't change this value, and should check
header_version everywhere when need max_mapnr, I will send a new
version to change it.
--
Thanks,
Jingbai Ma
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
