| Message ID | 20200821085011.28878-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] mm: Export flush_vm_area() to sync the PTEs upon construction | expand |
On Fri, Aug 21, 2020 at 1:50 AM Chris Wilson <chris@chris-wilson.co.uk> 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
Quoting Linus Torvalds (2020-08-21 13:41:03) > On Fri, Aug 21, 2020 at 1:50 AM Chris Wilson <chris@chris-wilson.co.uk> 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
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);
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 <pavel@ucw.cz> References: 2ba3e6947aed ("mm/vmalloc: track which page-table levels were modified") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Joerg Roedel <jroedel@suse.de> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Dave Airlie <airlied@redhat.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Pavel Machek <pavel@ucw.cz> Cc: <stable@vger.kernel.org> # v5.8+ --- drivers/gpu/drm/i915/gem/i915_gem_pages.c | 1 + 1 file changed, 1 insertion(+)