Messages in this thread | | | From | "Yu, Fenghua" <> | Date | Fri, 28 Aug 2009 13:08:30 -0700 | Subject | RE: [Patch 4/8] ia64: implement crashkernel=auto |
| |
>>>>> + unsigned long long ret; >>>>> + >>>>> + if (total_size >= GBYTES(4) && total_size < GBYTES(12)) >>>>> + ret = MBYTES(256); >>>>> + else if (total_size >= GBYTES(12) && total_size < GBYTES(128)) >>>>> + ret = MBYTES(512); >>>>> + else if (total_size >= GBYTES(128) && total_size < GBYTES(256)) >>>>> + ret = MBYTES(768); >>>>> + else if (total_size >= GBYTES(256) && total_size < GBYTES(378)) >>>>> + ret = MBYTES(1024); >>>>> + else if (total_size >= GBYTES(318) && total_size < GBYTES(512)) >>>>> + ret = MBYTES(1536); >>>>> + else if (total_size >= GBYTES(512) && total_size < GBYTES(768)) >>>>> + ret = MBYTES(2048); >>>>> + else >>>>> + ret = MBYTES(3072); >>>>> + if (!ia64_platform_is("sn2") && !ia64_platform_is("uv")) >>>>> + if (ret > GBYTES(4)) >>>>> + ret = ret / GBYTES(4) * MBYTES(256); >>>>> >>>>> >>>> This code doesn't make sense to me. What you really want is following >>>> >>> code, right? >>> >>>> + if (total_size > GBYTES(4)) >>>> + ret = total_size / GBYTES(4) * MBYTEST(256) >>>> >>>> >>>> >>> Nope. Check check_crashkernel_memory(). >>> >> >> This statement will be ALWAYS false: >> + if (ret > GBYTES(4)) >> since ret's max value is 3GB after the above total_size checks. >> >> So the code doesn't make sense. Did I miss something? >> > >Oops, in fact it was supposed to be: > > if (ret + base > GBYTES(4)) > //blah blah > >Hmm, but is it possible for ia64 to choose a base address above 1G? If >no, that code can be removed.
I think you don't need to consider the base here. This arch_default_crash_size() only returns size. Later on, ia64 setup_crashkernel() will find a base value in reserved region and check if the base value is under 4GB etc.
Thanks.
-Fenghua
| |