On Wed, Oct 31, 2018 at 12:00 PM Jerome Glisse <jglisse@xxxxxxxxxx> wrote: > > On Wed, Oct 31, 2018 at 05:08:23PM +0000, Robin Murphy wrote: > > Hi mm folks, > > > > I'm looking at ZONE_DEVICE support for arm64, and trying to make sense of a > > build failure has led me down the rabbit hole of pfn_t.h, and specifically > > __HAVE_ARCH_PTE_DEVMAP in this first instance. > > > > The failure itself is a link error in remove_migration_pte() due to a > > missing definition of pte_mkdevmap(), but I'm a little confused at the fact > > that it's explicitly declared without a definition, as if that breakage is > > deliberate. > > > > So, is the !__HAVE_ARCH_PTE_DEVMAP case actually expected to work? If not, > > then it seems to me that the relevant code could just be gated by > > CONFIG_ZONE_DEVICE directly to remove the confusion. If it is, though, then > > what should the generic definitions of p??_mkdevmap() be? I guess either way > > I still need to figure out the implications of _PAGE_DEVMAP at the arch end > > and whether/how arm64 should implement it, but given this initial hurdle > > it's not clear exactly where to go next. > > AFAIR you can get ZONE_DEVICE without PTE_DEVMAP, PTE_DEVMAP is an > optimization for pte_devmap() test ie being able to only have to > look at pte value to determine if it is a pte pointing to a ZONE_DEVICE > page versus needing to lookup the struct page. No, it's not an optimization it's required for get_user_pages(). The gup path uses the PTE_DEVMAP flag to determine that it needs to first pin a device hosting the pfn (get_dev_pagemap()), before pinning any associated pages. This allows device teardown operations to coordinate with in-flight gup requests. > As all architecture so far all have PTE_DEVMAP it might very well > be that the !_HAVE_ARCH_PTE_DEVMAP case is broken (either from the > start or because of changes made since it was added). It kind of > looks broken at least when i glance at it now (ie the default > pte_devmap() should lookup struct page and check if it is a ZONE > DEVICE page). That's the wrong way round because the 'struct page' object could be freed at any time if you don't have a dev_pagemap() reference. So, ZONE_DEVICE requires P??_DEVMAP. > So your life will be easier if you can do __HAVE_ARCH_PTE_DEVMAP > as you will not need to debug the !__HAVE_ARCH_PTE_DEVMAP case. Per above, no.
