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(_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:
> 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(_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
[PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page()
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(_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);
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);
@@ -237,7 +237,7 @@ static int check_partial_mappings(struct
drm_i915_gem_object *obj,
intel_gt_flush_ggtt_writes(_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);
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",
@@ -255,7 +255,7 @@ static int check_partial_mappings(struct
drm_i915_gem_object *obj,
}
*cpu = 0;
drm_clflush_virt_range(cpu, sizeof(*cpu));
- kunmap(p);
+ kunmap_local(cpu);
if (err)
return err;
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
index f8948de72036..743a414f86f3 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
@@ -743,7 +743,7 @@ static void swizzle_page(struct page *page)
char *vaddr;
int i;
- vaddr = kmap(page);
+ vaddr = kmap_local_page(page);
for (i = 0; i < PAGE_SIZE; i += 128) {
memcpy(temp, [i], 64);
@@ -751,7 +751,7 @@ static void swizzle_page(struct page *page)
memcpy([i + 64], temp, 64);
}
- kunmap(page);
+ kunmap_local(vaddr);
}
/**
diff --git a/drivers/gpu/drm/i915/gt/shmem_utils.c
b/drivers/gpu/drm/i915/gt/shmem_utils.c
index 0683b27a3890..cba4f210d093 100644
--- a/drivers/gpu/drm/i915/gt/shmem_utils.c
+++ b/drivers/gpu/drm/i915/gt/shmem_utils.c
@@ -104,7 +104,7 @@ static int __shmem_rw(struct file *file, loff_t off,
if (IS_ERR(page))
return PTR_ERR(page);
- vaddr = kmap(page);
+ vaddr = kmap_local_page(page);
if (write) {
memcpy(vaddr + offset_in_page(off), ptr, this);
set_page_dirty(page);
@@ -112,7 +112,7 @@ static int __shmem_rw(struct file *file, loff_t off,
memcpy(ptr, vaddr + offset_in_page(off), this);
}
mark_page_accessed(page);
-
