Re: [PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page()
On Mon, Dec 13, 2021 at 01:04:07AM -0800, Christoph Hellwig wrote: > On Fri, Dec 10, 2021 at 03:23:58PM -0800, ira.we...@intel.com wrote: > > - vaddr = kmap(page); > > + vaddr = kmap_local_page(page); > > memcpy(vaddr, data, len); > > - kunmap(page); > > + kunmap_local(vaddr); > > memcpy_to_page? Opps! Yea! David, Daniel, Do you prefer me to resent the entire series or reply to this message with a V2? Ira
Re: [PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page()
On Mon, Dec 13, 2021 at 02:39:59PM +0200, Ville Syrjälä wrote: > On Fri, Dec 10, 2021 at 03:23:58PM -0800, ira.we...@intel.com wrote: > > From: Ira Weiny > > > > kmap() is being deprecated and these usages are all local to the thread > > so there is no reason kmap_local_page() can't be used. > > > > Replace kmap() calls with kmap_local_page(). > > > > Signed-off-by: Ira Weiny > > --- > > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++-- > > drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8 > > drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++-- > > drivers/gpu/drm/i915/gt/shmem_utils.c | 4 ++-- > > drivers/gpu/drm/i915/i915_gem.c| 8 > > drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++-- > > 6 files changed, 16 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > > b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > > index d77da59fae04..fa8b820e14aa 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > > @@ -597,9 +597,9 @@ i915_gem_object_create_shmem_from_data(struct > > drm_i915_private *dev_priv, > > if (err < 0) > > goto fail; > > > > - vaddr = kmap(page); > > + vaddr = kmap_local_page(page); > > memcpy(vaddr, data, len); > > - kunmap(page); > > + kunmap_local(vaddr); > > > > err = pagecache_write_end(file, file->f_mapping, > > offset, len, len, > > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > > b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > > index 6d30cdfa80f3..e59e1725e29d 100644 > > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > > @@ -144,7 +144,7 @@ static int check_partial_mapping(struct > > drm_i915_gem_object *obj, > > intel_gt_flush_ggtt_writes(&to_i915(obj->base.dev)->gt); > > > > p = i915_gem_object_get_page(obj, offset >> PAGE_SHIFT); > > - cpu = kmap(p) + offset_in_page(offset); > > + cpu = kmap_local_page(p) + offset_in_page(offset); > > Does kunmap_local() do some magic to make it work even when you > don't pass in the same value you got from kmap_local_page()? Yes. It sounds like a patch like this would be nice to clarify? Ira diff --git a/include/linux/highmem-internal.h b/include/linux/highmem-internal.h index 0a0b2b09b1b8..fb2d3e033c01 100644 --- a/include/linux/highmem-internal.h +++ b/include/linux/highmem-internal.h @@ -246,6 +246,17 @@ do { \ __kunmap_atomic(__addr);\ } while (0) +/** + * kunmap_local - Unmap a page mapped via kmap_local_page(). + * @__addr: An address within the page mapped + * + * __addr is often an address returned from kmap_local_page(). However, + * this address can be any address within the mapped page. It does not need to + * be the exact address returned from kmap_local_page() + * + * Unmapping should be done in the reverse order of the mapping. See + * kmap_local_page() for details. + */ #define kunmap_local(__addr) \ do { \ BUILD_BUG_ON(__same_type((__addr), struct page *)); \
Re: [PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page()
On Fri, Dec 10, 2021 at 03:23:58PM -0800, ira.we...@intel.com wrote: > - vaddr = kmap(page); > + vaddr = kmap_local_page(page); > memcpy(vaddr, data, len); > - kunmap(page); > + kunmap_local(vaddr); memcpy_to_page?
Re: [PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page()
On Fri, Dec 10, 2021 at 03:23:58PM -0800, ira.we...@intel.com wrote: > From: Ira Weiny > > kmap() is being deprecated and these usages are all local to the thread > so there is no reason kmap_local_page() can't be used. > > Replace kmap() calls with kmap_local_page(). > > Signed-off-by: Ira Weiny > --- > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++-- > drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8 > drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++-- > drivers/gpu/drm/i915/gt/shmem_utils.c | 4 ++-- > drivers/gpu/drm/i915/i915_gem.c| 8 > drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++-- > 6 files changed, 16 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > index d77da59fae04..fa8b820e14aa 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > @@ -597,9 +597,9 @@ i915_gem_object_create_shmem_from_data(struct > drm_i915_private *dev_priv, > if (err < 0) > goto fail; > > - vaddr = kmap(page); > + vaddr = kmap_local_page(page); > memcpy(vaddr, data, len); > - kunmap(page); > + kunmap_local(vaddr); > > err = pagecache_write_end(file, file->f_mapping, > offset, len, len, > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > index 6d30cdfa80f3..e59e1725e29d 100644 > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c > @@ -144,7 +144,7 @@ static int check_partial_mapping(struct > drm_i915_gem_object *obj, > intel_gt_flush_ggtt_writes(&to_i915(obj->base.dev)->gt); > > p = i915_gem_object_get_page(obj, offset >> PAGE_SHIFT); > - cpu = kmap(p) + offset_in_page(offset); > + cpu = kmap_local_page(p) + offset_in_page(offset); Does kunmap_local() do some magic to make it work even when you don't pass in the same value you got from kmap_local_page()? > drm_clflush_virt_range(cpu, sizeof(*cpu)); > if (*cpu != (u32)page) { > pr_err("Partial view for %lu [%u] (offset=%llu, size=%u [%llu, > row size %u], fence=%d, tiling=%d, stride=%d) misalignment, expected write to > page (%llu + %u [0x%llx]) of 0x%x, found 0x%x\n", > @@ -162,7 +162,7 @@ static int check_partial_mapping(struct > drm_i915_gem_object *obj, > } > *cpu = 0; > drm_clflush_virt_range(cpu, sizeof(*cpu)); > - kunmap(p); > + kunmap_local(cpu); > > out: > __i915_vma_put(vma); -- Ville Syrjälä Intel