Re: [PATCH v2 03/13] mm: Add generic p?d_large() macros

[Steven Price]

On 01/03/2019 12:30, Kirill A. Shutemov wrote:
> On Fri, Mar 01, 2019 at 01:53:01PM +0200, Mike Rapoport wrote:
>> Him Kirill,
>>
>> On Fri, Feb 22, 2019 at 12:06:18AM +0300, Kirill A. Shutemov wrote:
>>> On Thu, Feb 21, 2019 at 05:16:46PM +0000, Steven Price wrote:
>>>>>> 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?
>>>
>>> These helpers are useful beyond pagewalker.
>>>
>>> Can we actually do some grinding and make *all* archs to provide correct
>>> helpers? Yes, it's tedious, but not that bad.
>>
>> Many architectures simply cannot support non-leaf entries at the higher
>> levels. I think letting the use a generic helper actually does make sense.
> 
> I disagree.
> 
> It's makes sense if the level doesn't exists on the arch.

This is what patch 24 [1] of the series does - if the level doesn't
exist then appropriate stubs are provided.

> But if the level exists, it will be less frugile to ask the arch to
> provide the helper. Even if it is dummy always-false.

The problem (as I see it), is we need a reliable set of p?d_large()
implementations to be able to walk arbitrary page tables. Either the
entire functionality of walking page tables without a VMA has to be an
opt-in per architecture, or we need to mandate that every architecture
provide these implementations.

I could provide an asm-generic header to provide a complete set of dummy
implementations for architectures that don't support large pages at all,
but that seems a bit overkill when most architectures only need to
define 2 or 3 implementations (the rest being provided by the
folded-levels automatically).

Thanks,

Steve

[1]
https://lore.kernel.org/lkml/20190227170608.27963-25-steven.price@xxxxxxx/