Re: [PATCH 2/4] drm/i915/gem: Sync the vmap PTEs upon construction
Quoting Linus Torvalds (2020-08-21 13:41:03) > On Fri, Aug 21, 2020 at 1:50 AM Chris Wilson wrote: > > > > Since synchronising the PTE after assignment is a manual step, use the > > newly exported interface to flush the PTE after assigning via > > alloc_vm_area(). > > This commit message doesn't make much sense to me. > > Are you talking about synchronizing the page directory structure > across processes after possibly creating new kernel page tables? > > Because that has nothing to do with the PTE. It's all about making > sure the _upper_ layers of the page directories are populated > everywhere.. > > The name seems off to me too - what are you "flushing"? (And yes, I > know about the flush_cache_vmap(), but that looks just bogus, since > any non-mapped area shouldn't have any virtual caches to begin with, > so I suspect that is just the crazy architectures being confused - > flush_cache_vmap() is a no-op on any sane architecture - and powerpc > that mis-uses it for other things). I was trying to mimic map_kernel_range() which does the arch_sync_kernel_mappings and flush_cache_vmap on top of the apply_to_page_range performed by alloc_vm_area, because buried away in there, on x86-32, is a set_pmd(). Since map_kernel_range() wrapped map_kernel_range_noflush(), flush seemed like the right verb. Joerg pointed out that the sync belonged to __apply_to_page_range and fixed it in situ. -Chris
Re: [PATCH 2/4] drm/i915/gem: Sync the vmap PTEs upon construction
On Fri, Aug 21, 2020 at 1:50 AM Chris Wilson wrote: > > Since synchronising the PTE after assignment is a manual step, use the > newly exported interface to flush the PTE after assigning via > alloc_vm_area(). This commit message doesn't make much sense to me. Are you talking about synchronizing the page directory structure across processes after possibly creating new kernel page tables? Because that has nothing to do with the PTE. It's all about making sure the _upper_ layers of the page directories are populated everywhere.. The name seems off to me too - what are you "flushing"? (And yes, I know about the flush_cache_vmap(), but that looks just bogus, since any non-mapped area shouldn't have any virtual caches to begin with, so I suspect that is just the crazy architectures being confused - flush_cache_vmap() is a no-op on any sane architecture - and powerpc that mis-uses it for other things). Linus
[PATCH 2/4] drm/i915/gem: Sync the vmap PTEs upon construction
Since synchronising the PTE after assignment is a manual step, use the newly exported interface to flush the PTE after assigning via alloc_vm_area(). Reported-by: Pavel Machek References: 2ba3e6947aed ("mm/vmalloc: track which page-table levels were modified") Signed-off-by: Chris Wilson Cc: Andrew Morton Cc: Joerg Roedel Cc: Linus Torvalds Cc: Dave Airlie Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: Pavel Machek Cc: # v5.8+ --- drivers/gpu/drm/i915/gem/i915_gem_pages.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c index 7050519c87a4..0fee67f34d74 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c @@ -304,6 +304,7 @@ static void *i915_gem_object_map(struct drm_i915_gem_object *obj, for_each_sgt_daddr(addr, iter, sgt) **ptes++ = iomap_pte(iomap, addr, pgprot); } + flush_vm_area(area); if (mem != stack) kvfree(mem); -- 2.20.1