Vivek Goyal <[EMAIL PROTECTED]> writes: > Hi Eric, > > This is a query regarding the x86_64 relocatable kernel patch in which > __pa() definition has been modified. > > http://marc.theaimsgroup.com/?l=linux-kernel&m=116380877430266&w=2 > > Previously __pa() could handle both linearly mapped region as well > as kernel text region. Now it can only handle only linearly mapped > region and for kernel text region one is supposed to call __pa_symbol(). > > But arch independent code can not call __pa_symbol() as this definition > is local to i386 and x86_64 arches. So is it acceptable to change > symantics of __pa()?
Yes. Other architectures have the same restriction on __pa, and virt_to_phys... At least they did last time I looked. > If yes, then we also need to change virt_to_phys() and virt_addr_valid() > macros. > > #define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT) > #define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT) > > These guys call __pa() unconditionally assuming __pa() can handle > both the address ranges. But with this change it is no more valid > and we found one problem in hibernate code where virt_to_page() is > being called for kernel text symbol "swsusp_header". > > In kernel/power/swap.c > rw_swap_page_sync(READ, swp_entry(root_swap, 0), > virt_to_page((unsigned long)&swsusp_header), NULL); > > Now virt_to_page() returns wrong information. Unless I'm confused about the semantics that is a bug in the swap suspend code. That code isn't really arch independent so no huge surprise it wasn't caught before. But ouch! > I can think of two possible solutions. > > - Restore the __pa() behaviour and live with increased cost of __pa() > operation (due to reading of a variable from memory) because of > relocatable kernel. > > - Modify dependent operations like virt_to_page() and virt_addr_valid() > to differentiate between two address spaces and either use __pa() > or __pa_symbol() accordingly. Though I am not very sure up to what > extent this will result in reduced cost of operation as virt_to_page() > is also being used at lots of places. > > Any thoughts? Yes. Double check that most of the callers to virt_to_page are not using it on a kernel text or data address. I could be wrong but I suspect you have empirically found the one that was wrong. I'm pretty certain if we go down the road of allowing that for kernel data next people will want those functions to work on vmalloc'd memory and on the fixmaps. They really are only defined on the kernel's linear region. Eric _______________________________________________ fastboot mailing list [email protected] https://lists.osdl.org/mailman/listinfo/fastboot
