On Thu, Apr 4, 2013 at 10:36 AM, Tejun Heo <tj@xxxxxxxxxx> wrote: > Hello, > > On Sat, Mar 09, 2013 at 10:44:30PM -0800, Yinghai Lu wrote: >> Now we have arch_pfn_mapped array, and max_low_pfn_mapped should not >> be used anymore. >> >> User should use arch_pfn_mapped or just 1UL<<(32-PAGE_SHIFT) instead. >> >> Only user is ACPI_INITRD_TABLE_OVERRIDE, and it should not use that, >> as later accessing is using early_ioremap(). Change to try to 4G below >> and then 4G above. > ... >> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c >> index 586e7e9..c08cdb6 100644 >> --- a/drivers/acpi/osl.c >> +++ b/drivers/acpi/osl.c >> @@ -624,9 +624,13 @@ void __init acpi_initrd_override(void *data, size_t size) >> if (table_nr == 0) >> return; >> >> - acpi_tables_addr = >> - memblock_find_in_range(0, max_low_pfn_mapped << PAGE_SHIFT, >> - all_tables_size, PAGE_SIZE); >> + /* under 4G at first, then above 4G */ >> + acpi_tables_addr = memblock_find_in_range(0, (1ULL<<32) - 1, >> + all_tables_size, PAGE_SIZE); >> + if (!acpi_tables_addr) >> + acpi_tables_addr = memblock_find_in_range(0, >> + ~(phys_addr_t)0, >> + all_tables_size, PAGE_SIZE); > > So, it's changing the allocation from <=4G to <=4G first and then >4G. > The only explanation given is "as later accessing is using > early_ioremap()", but I can't see why that can be a reason for that. > early_ioremap() doesn't care whether the given physaddr is under 4G or > not, it unconditionally maps it into fixmap, so whether the allocated > address is below or above 4G doesn't make any difference. > > Changing the allowed range of the allocation should be a separate > patch. It has some chance of its own breakage and the change itself > isn't really related to this one. Ok, will separate that "try above 4G" to another patch. > > Please try to elaborate the reasoning behind "why", so that readers of > the description don't have to deduce (oh well, guess) your intentions > behind the changes. As much as it would help the readers, it'd also > help you even more as you would have had to explicitly write something > like "the table is accessed with early_ioremap() so the address > doesn't need to be restricted under 4G; however, to avoid unnecessary > remappings, first try <= 4G and then > 4G." Then, you would be > compelled to check whether the statement you explicitly wrote is true, > which isn't in this case and you would also realize that the change > isn't trivial and doesn't really belong with this patch. By not doing > the due diligence, you're offloading what you should have done to > others, which isn't very nice. > > I think the descriptions are better in this posting than the last time > but it's still lacking, so, please putfff more effort into describing > the changes and reasoning behind them. ok. Thanks a lot. Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
