On 21/02/2019 14:57, Kirill A. Shutemov wrote: > On Thu, Feb 21, 2019 at 02:46:18PM +0000, Steven Price wrote: >> On 21/02/2019 14:28, Kirill A. Shutemov wrote: >>> On Thu, Feb 21, 2019 at 11:34:52AM +0000, Steven Price wrote: >>>> From: James Morse <james.morse@xxxxxxx> >>>> >>>> Exposing the pud/pgd levels of the page tables to walk_page_range() means >>>> we may come across the exotic large mappings that come with large areas >>>> of contiguous memory (such as the kernel's linear map). >>>> >>>> For architectures that don't provide p?d_large() macros, provided a >>>> does nothing default. >>> >>> Nak, sorry. >>> >>> Power will get broken by the patch. It has pmd_large() inline function, >>> that will be overwritten by the define from this patch. >>> >>> I believe it requires more ground work on arch side in general. >>> All architectures that has huge page support has to provide these helpers >>> (and matching defines) before you can use it in a generic code. >> >> Sorry about that, I had compile tested on power, but obviously not the >> right config to actually see the breakage. > > I don't think you'll catch it at compile-time. It would silently override > the helper with always-false. Ah, that might explain why I missed it. >> I'll do some grepping - hopefully this is just a case of exposing the >> functions/defines that already exist for those architectures. > > I see the same type of breakage on s390 and sparc. > >> Note that in terms of the new page walking code, these new defines are >> only used when walking a page table without a VMA (which isn't currently >> done), so architectures which don't use p?d_large currently will work >> fine with the generic versions. They only need to provide meaningful >> definitions when switching to use the walk-without-a-VMA functionality. > > How other architectures would know that they need to provide the helpers > to get walk-without-a-VMA functionality? This looks very fragile to me. Yes, you've got a good point there. This would apply to the p?d_large macros as well - any arch which (inadvertently) uses the generic version is likely to be fragile/broken. I think probably the best option here is to scrap the generic versions altogether and simply introduce a ARCH_HAS_PXD_LARGE config option which would enable the new functionality to those arches that opt-in. Do you think this would be less fragile? Thanks, Steve
