From: Zhao Liu <[email protected]>
Hi all,
I refreshed this v3 by rebasing v2 [1] on the commit 968f35f4ab1c
("Merge tag 'v6.7-rc3-smb3-client-fixes' of git://git.samba.org/sfrench/
cifs-2.6").
Based on the current code, I rechecked the substitutions in v2 and they
still stand and are valid, so no code change in v3.
Thanks for all the review! And sorry v2 was missed, I'll pay more
attention to this v3.
Purpose of This Patchset
========================
The purpose of this pacthset is to replace all uses of kmap_atomic() in
i915 with kmap_local_page() because the use of kmap_atomic() is being
deprecated in favor of kmap_local_page()[2]. And 92b64bd (mm/highmem:
add notes about conversions from kmap{,_atomic}()) has declared the
deprecation of kmap_atomic().
Motivation for Deprecating kmap_atomic() and Using kmap_local_page()
====================================================================
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
Patch summary
=============
Patch 1, 4-6 and 8-9 replace kmap_atomic()/kunmap_atomic() with
kmap_local_page()/kunmap_local() directly. With these local
mappings, the page faults and preemption are allowed.
Patch 2 and 7 use memcpy_from_page() and memcpy_to_page() to replace
kmap_atomic()/kunmap_atomic(). These two variants of memcpy()
are based on the local mapping, so page faults and preemption
are also allowed in these two interfaces.
Patch 3 replaces kmap_atomic()/kunmap_atomic() with kmap_local_page()/
kunmap_local() and also disable page fault since the for special
handling (pls see the commit message).
Reference
=========
[1]: https://lore.kernel.org/all/[email protected]/
[2]: https://lore.kernel.org/all/[email protected]
Thanks and Best Regards,
Zhao
---
Changlog:
Changes since v2:
* Rebased on 968f35f4ab1c ("Merge tag 'v6.7-rc3-smb3-client-fixes' of
git://git.samba.org/sfrench/cifs-2.6").
* Removed changelog (of v2) in commit message.
* Fixed typo in cover letter (Fabio).
* Added Reviewed-by tags from Ira and Fabio.
Changes since v1:
* Dropped hot plug related description in commit message since it has
nothing to do with kmap_local_page().
* Emphasized the motivation for using kmap_local_page() in commit
message.
* Rebased patch 1 on f47e630 (drm/i915/gem: Typecheck page lookups) to
keep the "idx" variable of type pgoff_t here.
* Used memcpy_from_page() and memcpy_to_page() to replace
kmap_local_page() + memcpy() in patch 2.
---
Zhao Liu (9):
drm/i915: Use kmap_local_page() in gem/i915_gem_object.c
drm/i915: Use memcpy_[from/to]_page() in gem/i915_gem_pyhs.c
drm/i915: Use kmap_local_page() in gem/i915_gem_shmem.c
drm/i915: Use kmap_local_page() in gem/selftests/huge_pages.c
drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_coherency.c
drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_context.c
drm/i915: Use memcpy_from_page() in gt/uc/intel_uc_fw.c
drm/i915: Use kmap_local_page() in i915_cmd_parser.c
drm/i915: Use kmap_local_page() in gem/i915_gem_execbuffer.c
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +++++-----
drivers/gpu/drm/i915/gem/i915_gem_object.c | 8 +++-----
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 10 ++--------
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 ++++--
drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 6 +++---
.../gpu/drm/i915/gem/selftests/i915_gem_coherency.c | 12 ++++--------
.../gpu/drm/i915/gem/selftests/i915_gem_context.c | 8 ++++----
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 +----
drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++--
9 files changed, 28 insertions(+), 41 deletions(-)
--
2.34.1
From: Zhao Liu <[email protected]>
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1].
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults or preemption disables.
In drm/i915/gem/i915_gem_shmem.c, the function shmem_pwrite() need to
disable pagefault to eliminate the potential recursion fault[2]. But
here __copy_from_user_inatomic() doesn't need to disable preemption and
local mapping is valid for sched in/out.
So it can use kmap_local_page() / kunmap_local() with
pagefault_disable() / pagefault_enable() to replace atomic mapping.
[1]: https://lore.kernel.org/all/[email protected]
[2]: https://patchwork.freedesktop.org/patch/295840/
Suggested-by: Ira Weiny <[email protected]>
Suggested-by: Fabio M. De Francesco <[email protected]>
Signed-off-by: Zhao Liu <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
Reviewed-by: Fabio M. De Francesco <[email protected]>
---
Suggested by credits:
Ira: Referred to his suggestions about keeping pagefault_disable().
Fabio: Referred to his description about why kmap_local_page() should
be preferred.
---
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 73a4a4eb29e0..38b72d86560f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -485,11 +485,13 @@ shmem_pwrite(struct drm_i915_gem_object *obj,
if (err < 0)
return err;
- vaddr = kmap_atomic(page);
+ vaddr = kmap_local_page(page);
+ pagefault_disable();
unwritten = __copy_from_user_inatomic(vaddr + pg,
user_data,
len);
- kunmap_atomic(vaddr);
+ pagefault_enable();
+ kunmap_local(vaddr);
err = aops->write_end(obj->base.filp, mapping, offset, len,
len - unwritten, page, data);
--
2.34.1
From: Zhao Liu <[email protected]>
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
There're 2 reasons why i915_gem_object_read_from_page_kmap() doesn't
need to disable pagefaults and preemption for mapping:
1. The flush operation is safe. In drm/i915/gem/i915_gem_object.c,
i915_gem_object_read_from_page_kmap() calls drm_clflush_virt_range() to
use CLFLUSHOPT or WBINVD to flush. Since CLFLUSHOPT is global on x86
and WBINVD is called on each cpu in drm_clflush_virt_range(), the flush
operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, i915_gem_object_read_from_page_kmap() is a function where
the use of kmap_local_page() in place of kmap_atomic() is correctly
suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
And remove the redundant variable that stores the address of the mapped
page since kunmap_local() can accept any pointer within the page.
[1]: https://lore.kernel.org/all/[email protected]
Suggested-by: Dave Hansen <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Suggested-by: Fabio M. De Francesco <[email protected]>
Signed-off-by: Zhao Liu <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
Reviewed-by: Fabio M. De Francesco <[email protected]>
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index c26d87555825..a2a7e5005415 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -493,17 +493,15 @@ static void
i915_gem_object_read_from_page_kmap(struct drm_i915_gem_object *obj, u64 offset, void *dst, int size)
{
pgoff_t idx = offset >> PAGE_SHIFT;
- void *src_map;
void *src_ptr;
- src_map = kmap_atomic(i915_gem_object_get_page(obj, idx));
-
- src_ptr = src_map + offset_in_page(offset);
+ src_ptr = kmap_local_page(i915_gem_object_get_page(obj, idx))
+ + offset_in_page(offset);
if (!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ))
drm_clflush_virt_range(src_ptr, size);
memcpy(dst, src_ptr, size);
- kunmap_atomic(src_map);
+ kunmap_local(src_ptr);
}
static void
--
2.34.1
From: Zhao Liu <[email protected]>
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() + memcpy() to memcpy_[from/to]_page(), which use
kmap_local_page() to build local mapping and then do memcpy().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
In drm/i915/gem/i915_gem_phys.c, the functions
i915_gem_object_get_pages_phys() and i915_gem_object_put_pages_phys()
don't need to disable pagefaults and preemption for mapping because of
2 reasons:
1. The flush operation is safe. In drm/i915/gem/i915_gem_object.c,
i915_gem_object_get_pages_phys() and i915_gem_object_put_pages_phys()
calls drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush.
Since CLFLUSHOPT is global on x86 and WBINVD is called on each cpu in
drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, i915_gem_object_get_pages_phys() and
i915_gem_object_put_pages_phys() are two functions where the uses of
local mappings in place of atomic mappings are correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() + memcpy() to
memcpy_from_page() and memcpy_to_page().
[1]: https://lore.kernel.org/all/[email protected]
Suggested-by: Dave Hansen <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Suggested-by: Fabio M. De Francesco <[email protected]>
Signed-off-by: Zhao Liu <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
Reviewed-by: Fabio M. De Francesco <[email protected]>
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
index 5df128e2f4dc..ef85c6dc9fd5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
@@ -65,16 +65,13 @@ static int i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
dst = vaddr;
for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
struct page *page;
- void *src;
page = shmem_read_mapping_page(mapping, i);
if (IS_ERR(page))
goto err_st;
- src = kmap_atomic(page);
- memcpy(dst, src, PAGE_SIZE);
+ memcpy_from_page(dst, page, 0, PAGE_SIZE);
drm_clflush_virt_range(dst, PAGE_SIZE);
- kunmap_atomic(src);
put_page(page);
dst += PAGE_SIZE;
@@ -113,16 +110,13 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj,
for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
struct page *page;
- char *dst;
page = shmem_read_mapping_page(mapping, i);
if (IS_ERR(page))
continue;
- dst = kmap_atomic(page);
drm_clflush_virt_range(src, PAGE_SIZE);
- memcpy(dst, src, PAGE_SIZE);
- kunmap_atomic(dst);
+ memcpy_to_page(page, 0, src, PAGE_SIZE);
set_page_dirty(page);
if (obj->mm.madv == I915_MADV_WILLNEED)
--
2.34.1
From: Zhao Liu <[email protected]>
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults or preemption disables.
In drm/i915/gem/selftests/huge_pages.c, function __cpu_check_shmem()
mainly uses mapping to flush cache and check the value. There're
2 reasons why __cpu_check_shmem() doesn't need to disable pagefaults
and preemption for mapping:
1. The flush operation is safe. Function __cpu_check_shmem() calls
drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush. Since
CLFLUSHOPT is global on x86 and WBINVD is called on each cpu in
drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, __cpu_check_shmem() is a function where the use of
kmap_local_page() in place of kmap_atomic() is correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/[email protected]
Suggested-by: Dave Hansen <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Suggested-by: Fabio M. De Francesco <[email protected]>
Signed-off-by: Zhao Liu <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
Reviewed-by: Fabio M. De Francesco <[email protected]>
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index 6b9f6cf50bf6..c9e6d77abab0 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1082,7 +1082,7 @@ __cpu_check_shmem(struct drm_i915_gem_object *obj, u32 dword, u32 val)
goto err_unlock;
for (n = 0; n < obj->base.size >> PAGE_SHIFT; ++n) {
- u32 *ptr = kmap_atomic(i915_gem_object_get_page(obj, n));
+ u32 *ptr = kmap_local_page(i915_gem_object_get_page(obj, n));
if (needs_flush & CLFLUSH_BEFORE)
drm_clflush_virt_range(ptr, PAGE_SIZE);
@@ -1090,12 +1090,12 @@ __cpu_check_shmem(struct drm_i915_gem_object *obj, u32 dword, u32 val)
if (ptr[dword] != val) {
pr_err("n=%lu ptr[%u]=%u, val=%u\n",
n, dword, ptr[dword], val);
- kunmap_atomic(ptr);
+ kunmap_local(ptr);
err = -EINVAL;
break;
}
- kunmap_atomic(ptr);
+ kunmap_local(ptr);
}
i915_gem_object_finish_access(obj);
--
2.34.1
From: Zhao Liu <[email protected]>
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults or preemption disables.
In drm/i915/gt/uc/intel_us_fw.c, the function intel_uc_fw_copy_rsa()
just use the mapping to do memory copy so it doesn't need to disable
pagefaults and preemption for mapping. Thus the local mapping without
atomic context (not disable pagefaults / preemption) is enough.
Therefore, intel_uc_fw_copy_rsa() is a function where the use of
memcpy_from_page() with kmap_local_page() in place of memcpy() with
kmap_atomic() is correctly suited.
Convert the calls of memcpy() with kmap_atomic() / kunmap_atomic() to
memcpy_from_page() which uses local mapping to copy.
[1]: https://lore.kernel.org/all/[email protected]/T/#u
Suggested-by: Ira Weiny <[email protected]>
Suggested-by: Fabio M. De Francesco <[email protected]>
Signed-off-by: Zhao Liu <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
Reviewed-by: Fabio M. De Francesco <[email protected]>
---
Suggested by credits:
Ira: Referred to his task document and suggestions about using
memcpy_from_page() directly.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 362639162ed6..756093eaf2ad 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -1343,16 +1343,13 @@ size_t intel_uc_fw_copy_rsa(struct intel_uc_fw *uc_fw, void *dst, u32 max_len)
for_each_sgt_page(page, iter, uc_fw->obj->mm.pages) {
u32 len = min_t(u32, size, PAGE_SIZE - offset);
- void *vaddr;
if (idx > 0) {
idx--;
continue;
}
- vaddr = kmap_atomic(page);
- memcpy(dst, vaddr + offset, len);
- kunmap_atomic(vaddr);
+ memcpy_from_page(dst, page, offset, len);
offset = 0;
dst += len;
--
2.34.1
From: Zhao Liu <[email protected]>
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration)..
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults or preemption disables.
In drm/i915/gem/selftests/i915_gem_coherency.c, functions cpu_set()
and cpu_get() mainly uses mapping to flush cache and assign the value.
There're 2 reasons why cpu_set() and cpu_get() don't need to disable
pagefaults and preemption for mapping:
1. The flush operation is safe. cpu_set() and cpu_get() call
drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush. Since
CLFLUSHOPT is global on x86 and WBINVD is called on each cpu in
drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, cpu_set() and cpu_get() are functions where the use of
kmap_local_page() in place of kmap_atomic() is correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/[email protected]
Suggested-by: Dave Hansen <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Suggested-by: Fabio M. De Francesco <[email protected]>
Signed-off-by: Zhao Liu <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
Reviewed-by: Fabio M. De Francesco <[email protected]>
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
.../gpu/drm/i915/gem/selftests/i915_gem_coherency.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
index 3bef1beec7cb..beeb3e12eccc 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
@@ -24,7 +24,6 @@ static int cpu_set(struct context *ctx, unsigned long offset, u32 v)
{
unsigned int needs_clflush;
struct page *page;
- void *map;
u32 *cpu;
int err;
@@ -34,8 +33,7 @@ static int cpu_set(struct context *ctx, unsigned long offset, u32 v)
goto out;
page = i915_gem_object_get_page(ctx->obj, offset >> PAGE_SHIFT);
- map = kmap_atomic(page);
- cpu = map + offset_in_page(offset);
+ cpu = kmap_local_page(page) + offset_in_page(offset);
if (needs_clflush & CLFLUSH_BEFORE)
drm_clflush_virt_range(cpu, sizeof(*cpu));
@@ -45,7 +43,7 @@ static int cpu_set(struct context *ctx, unsigned long offset, u32 v)
if (needs_clflush & CLFLUSH_AFTER)
drm_clflush_virt_range(cpu, sizeof(*cpu));
- kunmap_atomic(map);
+ kunmap_local(cpu);
i915_gem_object_finish_access(ctx->obj);
out:
@@ -57,7 +55,6 @@ static int cpu_get(struct context *ctx, unsigned long offset, u32 *v)
{
unsigned int needs_clflush;
struct page *page;
- void *map;
u32 *cpu;
int err;
@@ -67,15 +64,14 @@ static int cpu_get(struct context *ctx, unsigned long offset, u32 *v)
goto out;
page = i915_gem_object_get_page(ctx->obj, offset >> PAGE_SHIFT);
- map = kmap_atomic(page);
- cpu = map + offset_in_page(offset);
+ cpu = kmap_local_page(page) + offset_in_page(offset);
if (needs_clflush & CLFLUSH_BEFORE)
drm_clflush_virt_range(cpu, sizeof(*cpu));
*v = *cpu;
- kunmap_atomic(map);
+ kunmap_local(cpu);
i915_gem_object_finish_access(ctx->obj);
out:
--
2.34.1
From: Zhao Liu <[email protected]>
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption.
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults or preemption disables.
In drm/i915/gem/selftests/i915_gem_context.c, functions cpu_fill() and
cpu_check() mainly uses mapping to flush cache and check/assign the
value.
There're 2 reasons why cpu_fill() and cpu_check() don't need to disable
pagefaults and preemption for mapping:
1. The flush operation is safe. cpu_fill() and cpu_check() call
drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush. Since
CLFLUSHOPT is global on x86 and WBINVD is called on each cpu in
drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, cpu_fill() and cpu_check() are functions where the use of
kmap_local_page() in place of kmap_atomic() is correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/[email protected]
Suggested-by: Dave Hansen <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Suggested-by: Fabio M. De Francesco <[email protected]>
Signed-off-by: Zhao Liu <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
Reviewed-by: Fabio M. De Francesco <[email protected]>
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index 7021b6e9b219..89d4dc8b60c6 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -489,12 +489,12 @@ static int cpu_fill(struct drm_i915_gem_object *obj, u32 value)
for (n = 0; n < real_page_count(obj); n++) {
u32 *map;
- map = kmap_atomic(i915_gem_object_get_page(obj, n));
+ map = kmap_local_page(i915_gem_object_get_page(obj, n));
for (m = 0; m < DW_PER_PAGE; m++)
map[m] = value;
if (!has_llc)
drm_clflush_virt_range(map, PAGE_SIZE);
- kunmap_atomic(map);
+ kunmap_local(map);
}
i915_gem_object_finish_access(obj);
@@ -520,7 +520,7 @@ static noinline int cpu_check(struct drm_i915_gem_object *obj,
for (n = 0; n < real_page_count(obj); n++) {
u32 *map, m;
- map = kmap_atomic(i915_gem_object_get_page(obj, n));
+ map = kmap_local_page(i915_gem_object_get_page(obj, n));
if (needs_flush & CLFLUSH_BEFORE)
drm_clflush_virt_range(map, PAGE_SIZE);
@@ -546,7 +546,7 @@ static noinline int cpu_check(struct drm_i915_gem_object *obj,
}
out_unmap:
- kunmap_atomic(map);
+ kunmap_local(map);
if (err)
break;
}
--
2.34.1
From: Zhao Liu <[email protected]>
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
There're 2 reasons why function copy_batch() doesn't need to disable
pagefaults and preemption for mapping:
1. The flush operation is safe. In i915_cmd_parser.c, copy_batch() calls
drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush.
Since CLFLUSHOPT is global on x86 and WBINVD is called on each cpu
in drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, copy_batch() is a function where the use of
kmap_local_page() in place of kmap_atomic() is correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/[email protected]
Suggested-by: Dave Hansen <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Suggested-by: Fabio M. De Francesco <[email protected]>
Signed-off-by: Zhao Liu <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
Reviewed-by: Fabio M. De Francesco <[email protected]>
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index ddf49c2dbb91..2905df83e180 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1211,11 +1211,11 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
for (n = offset >> PAGE_SHIFT; remain; n++) {
int len = min(remain, PAGE_SIZE - x);
- src = kmap_atomic(i915_gem_object_get_page(src_obj, n));
+ src = kmap_local_page(i915_gem_object_get_page(src_obj, n));
if (src_needs_clflush)
drm_clflush_virt_range(src + x, len);
memcpy(ptr, src + x, len);
- kunmap_atomic(src);
+ kunmap_local(src);
ptr += len;
remain -= len;
--
2.34.1
From: Zhao Liu <[email protected]>
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the calls from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
In i915_gem_execbuffer.c, eb->reloc_cache.vaddr is mapped by
kmap_atomic() in eb_relocate_entry(), and is unmapped by
kunmap_atomic() in reloc_cache_reset().
And this mapping/unmapping occurs in two places: one is in
eb_relocate_vma(), and another is in eb_relocate_vma_slow().
The function eb_relocate_vma() or eb_relocate_vma_slow() doesn't
need to disable pagefaults and preemption during the above mapping/
unmapping.
So it can simply use kmap_local_page() / kunmap_local() that can
instead do the mapping / unmapping regardless of the context.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/[email protected]
Suggested-by: Ira Weiny <[email protected]>
Suggested-by: Fabio M. De Francesco <[email protected]>
Signed-off-by: Zhao Liu <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
Reviewed-by: Fabio M. De Francesco <[email protected]>
---
Suggested by credits:
Ira: Referred to his task document, review comments.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 683fd8d3151c..18b0f3117074 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1156,7 +1156,7 @@ static void reloc_cache_unmap(struct reloc_cache *cache)
vaddr = unmask_page(cache->vaddr);
if (cache->vaddr & KMAP)
- kunmap_atomic(vaddr);
+ kunmap_local(vaddr);
else
io_mapping_unmap_atomic((void __iomem *)vaddr);
}
@@ -1172,7 +1172,7 @@ static void reloc_cache_remap(struct reloc_cache *cache,
if (cache->vaddr & KMAP) {
struct page *page = i915_gem_object_get_page(obj, cache->page);
- vaddr = kmap_atomic(page);
+ vaddr = kmap_local_page(page);
cache->vaddr = unmask_flags(cache->vaddr) |
(unsigned long)vaddr;
} else {
@@ -1202,7 +1202,7 @@ static void reloc_cache_reset(struct reloc_cache *cache, struct i915_execbuffer
if (cache->vaddr & CLFLUSH_AFTER)
mb();
- kunmap_atomic(vaddr);
+ kunmap_local(vaddr);
i915_gem_object_finish_access(obj);
} else {
struct i915_ggtt *ggtt = cache_to_ggtt(cache);
@@ -1234,7 +1234,7 @@ static void *reloc_kmap(struct drm_i915_gem_object *obj,
struct page *page;
if (cache->vaddr) {
- kunmap_atomic(unmask_page(cache->vaddr));
+ kunmap_local(unmask_page(cache->vaddr));
} else {
unsigned int flushes;
int err;
@@ -1256,7 +1256,7 @@ static void *reloc_kmap(struct drm_i915_gem_object *obj,
if (!obj->mm.dirty)
set_page_dirty(page);
- vaddr = kmap_atomic(page);
+ vaddr = kmap_local_page(page);
cache->vaddr = unmask_flags(cache->vaddr) | (unsigned long)vaddr;
cache->page = pageno;
--
2.34.1
Hi maintainers,
Just kindly ping.
May I ask if this refresh version could be merged into the next tree of
the i915?
Thanks,
Zhao
On Sun, Dec 03, 2023 at 09:29:38PM +0800, Zhao Liu wrote:
> Date: Sun, 3 Dec 2023 21:29:38 +0800
> From: Zhao Liu <[email protected]>
> Subject: [PATCH v3 0/9] drm/i915: Replace kmap_atomic() with
> kmap_local_page()
> X-Mailer: git-send-email 2.34.1
>
> From: Zhao Liu <[email protected]>
>
> Hi all,
>
> I refreshed this v3 by rebasing v2 [1] on the commit 968f35f4ab1c
> ("Merge tag 'v6.7-rc3-smb3-client-fixes' of git://git.samba.org/sfrench/
> cifs-2.6").
>
> Based on the current code, I rechecked the substitutions in v2 and they
> still stand and are valid, so no code change in v3.
>
> Thanks for all the review! And sorry v2 was missed, I'll pay more
> attention to this v3.
>
>
> Purpose of This Patchset
> ========================
>
> The purpose of this pacthset is to replace all uses of kmap_atomic() in
> i915 with kmap_local_page() because the use of kmap_atomic() is being
> deprecated in favor of kmap_local_page()[2]. And 92b64bd (mm/highmem:
> add notes about conversions from kmap{,_atomic}()) has declared the
> deprecation of kmap_atomic().
>
>
> Motivation for Deprecating kmap_atomic() and Using kmap_local_page()
> ====================================================================
>
> The main difference between atomic and local mappings is that local
> mappings doesn't disable page faults or preemption (the preemption is
> disabled for !PREEMPT_RT case, otherwise it only disables migration).
>
> With kmap_local_page(), we can avoid the often unwanted side effect of
> unnecessary page faults and preemption disables.
>
>
> Patch summary
> =============
>
> Patch 1, 4-6 and 8-9 replace kmap_atomic()/kunmap_atomic() with
> kmap_local_page()/kunmap_local() directly. With these local
> mappings, the page faults and preemption are allowed.
>
> Patch 2 and 7 use memcpy_from_page() and memcpy_to_page() to replace
> kmap_atomic()/kunmap_atomic(). These two variants of memcpy()
> are based on the local mapping, so page faults and preemption
> are also allowed in these two interfaces.
>
> Patch 3 replaces kmap_atomic()/kunmap_atomic() with kmap_local_page()/
> kunmap_local() and also disable page fault since the for special
> handling (pls see the commit message).
>
>
> Reference
> =========
>
> [1]: https://lore.kernel.org/all/[email protected]/
> [2]: https://lore.kernel.org/all/[email protected]
>
>
> Thanks and Best Regards,
> Zhao
>
> ---
> Changlog:
>
> Changes since v2:
> * Rebased on 968f35f4ab1c ("Merge tag 'v6.7-rc3-smb3-client-fixes' of
> git://git.samba.org/sfrench/cifs-2.6").
> * Removed changelog (of v2) in commit message.
> * Fixed typo in cover letter (Fabio).
> * Added Reviewed-by tags from Ira and Fabio.
>
> Changes since v1:
> * Dropped hot plug related description in commit message since it has
> nothing to do with kmap_local_page().
> * Emphasized the motivation for using kmap_local_page() in commit
> message.
> * Rebased patch 1 on f47e630 (drm/i915/gem: Typecheck page lookups) to
> keep the "idx" variable of type pgoff_t here.
> * Used memcpy_from_page() and memcpy_to_page() to replace
> kmap_local_page() + memcpy() in patch 2.
>
> ---
> Zhao Liu (9):
> drm/i915: Use kmap_local_page() in gem/i915_gem_object.c
> drm/i915: Use memcpy_[from/to]_page() in gem/i915_gem_pyhs.c
> drm/i915: Use kmap_local_page() in gem/i915_gem_shmem.c
> drm/i915: Use kmap_local_page() in gem/selftests/huge_pages.c
> drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_coherency.c
> drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_context.c
> drm/i915: Use memcpy_from_page() in gt/uc/intel_uc_fw.c
> drm/i915: Use kmap_local_page() in i915_cmd_parser.c
> drm/i915: Use kmap_local_page() in gem/i915_gem_execbuffer.c
>
> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +++++-----
> drivers/gpu/drm/i915/gem/i915_gem_object.c | 8 +++-----
> drivers/gpu/drm/i915/gem/i915_gem_phys.c | 10 ++--------
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 ++++--
> drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 6 +++---
> .../gpu/drm/i915/gem/selftests/i915_gem_coherency.c | 12 ++++--------
> .../gpu/drm/i915/gem/selftests/i915_gem_context.c | 8 ++++----
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 +----
> drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++--
> 9 files changed, 28 insertions(+), 41 deletions(-)
>
> --
> 2.34.1
>
Hi Zhao,
On 14/12/2023 13:19, Zhao Liu wrote:
> Hi maintainers,
>
> Just kindly ping.
> May I ask if this refresh version could be merged into the next tree of
> the i915?
I certainly spotted your series last week or so but then it slipped my
mind to go through it. Should be able to go through it today or tomorrow.
Regards,
Tvrtko
>
> Thanks,
> Zhao
>
> On Sun, Dec 03, 2023 at 09:29:38PM +0800, Zhao Liu wrote:
>> Date: Sun, 3 Dec 2023 21:29:38 +0800
>> From: Zhao Liu <[email protected]>
>> Subject: [PATCH v3 0/9] drm/i915: Replace kmap_atomic() with
>> kmap_local_page()
>> X-Mailer: git-send-email 2.34.1
>>
>> From: Zhao Liu <[email protected]>
>>
>> Hi all,
>>
>> I refreshed this v3 by rebasing v2 [1] on the commit 968f35f4ab1c
>> ("Merge tag 'v6.7-rc3-smb3-client-fixes' of git://git.samba.org/sfrench/
>> cifs-2.6").
>>
>> Based on the current code, I rechecked the substitutions in v2 and they
>> still stand and are valid, so no code change in v3.
>>
>> Thanks for all the review! And sorry v2 was missed, I'll pay more
>> attention to this v3.
>>
>>
>> Purpose of This Patchset
>> ========================
>>
>> The purpose of this pacthset is to replace all uses of kmap_atomic() in
>> i915 with kmap_local_page() because the use of kmap_atomic() is being
>> deprecated in favor of kmap_local_page()[2]. And 92b64bd (mm/highmem:
>> add notes about conversions from kmap{,_atomic}()) has declared the
>> deprecation of kmap_atomic().
>>
>>
>> Motivation for Deprecating kmap_atomic() and Using kmap_local_page()
>> ====================================================================
>>
>> The main difference between atomic and local mappings is that local
>> mappings doesn't disable page faults or preemption (the preemption is
>> disabled for !PREEMPT_RT case, otherwise it only disables migration).
>>
>> With kmap_local_page(), we can avoid the often unwanted side effect of
>> unnecessary page faults and preemption disables.
>>
>>
>> Patch summary
>> =============
>>
>> Patch 1, 4-6 and 8-9 replace kmap_atomic()/kunmap_atomic() with
>> kmap_local_page()/kunmap_local() directly. With these local
>> mappings, the page faults and preemption are allowed.
>>
>> Patch 2 and 7 use memcpy_from_page() and memcpy_to_page() to replace
>> kmap_atomic()/kunmap_atomic(). These two variants of memcpy()
>> are based on the local mapping, so page faults and preemption
>> are also allowed in these two interfaces.
>>
>> Patch 3 replaces kmap_atomic()/kunmap_atomic() with kmap_local_page()/
>> kunmap_local() and also disable page fault since the for special
>> handling (pls see the commit message).
>>
>>
>> Reference
>> =========
>>
>> [1]: https://lore.kernel.org/all/[email protected]/
>> [2]: https://lore.kernel.org/all/[email protected]
>>
>>
>> Thanks and Best Regards,
>> Zhao
>>
>> ---
>> Changlog:
>>
>> Changes since v2:
>> * Rebased on 968f35f4ab1c ("Merge tag 'v6.7-rc3-smb3-client-fixes' of
>> git://git.samba.org/sfrench/cifs-2.6").
>> * Removed changelog (of v2) in commit message.
>> * Fixed typo in cover letter (Fabio).
>> * Added Reviewed-by tags from Ira and Fabio.
>>
>> Changes since v1:
>> * Dropped hot plug related description in commit message since it has
>> nothing to do with kmap_local_page().
>> * Emphasized the motivation for using kmap_local_page() in commit
>> message.
>> * Rebased patch 1 on f47e630 (drm/i915/gem: Typecheck page lookups) to
>> keep the "idx" variable of type pgoff_t here.
>> * Used memcpy_from_page() and memcpy_to_page() to replace
>> kmap_local_page() + memcpy() in patch 2.
>>
>> ---
>> Zhao Liu (9):
>> drm/i915: Use kmap_local_page() in gem/i915_gem_object.c
>> drm/i915: Use memcpy_[from/to]_page() in gem/i915_gem_pyhs.c
>> drm/i915: Use kmap_local_page() in gem/i915_gem_shmem.c
>> drm/i915: Use kmap_local_page() in gem/selftests/huge_pages.c
>> drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_coherency.c
>> drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_context.c
>> drm/i915: Use memcpy_from_page() in gt/uc/intel_uc_fw.c
>> drm/i915: Use kmap_local_page() in i915_cmd_parser.c
>> drm/i915: Use kmap_local_page() in gem/i915_gem_execbuffer.c
>>
>> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +++++-----
>> drivers/gpu/drm/i915/gem/i915_gem_object.c | 8 +++-----
>> drivers/gpu/drm/i915/gem/i915_gem_phys.c | 10 ++--------
>> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 ++++--
>> drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 6 +++---
>> .../gpu/drm/i915/gem/selftests/i915_gem_coherency.c | 12 ++++--------
>> .../gpu/drm/i915/gem/selftests/i915_gem_context.c | 8 ++++----
>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 +----
>> drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++--
>> 9 files changed, 28 insertions(+), 41 deletions(-)
>>
>> --
>> 2.34.1
>>
On 14/12/2023 13:45, Tvrtko Ursulin wrote:
>
> Hi Zhao,
>
> On 14/12/2023 13:19, Zhao Liu wrote:
>> Hi maintainers,
>>
>> Just kindly ping.
>> May I ask if this refresh version could be merged into the next tree of
>> the i915?
>
> I certainly spotted your series last week or so but then it slipped my
> mind to go through it. Should be able to go through it today or tomorrow.
It all looks good to me. I only needed to queue a re-test in our CI
since v3 failed BAT, but pretty sure it wasn't at fault. Once I am
satisfied with the results I will merge the series. Thanks for the
cleanups and your patience!
Regards,
Tvrtko
> Regards,
>
> Tvrtko
>
>>
>> Thanks,
>> Zhao
>>
>> On Sun, Dec 03, 2023 at 09:29:38PM +0800, Zhao Liu wrote:
>>> Date: Sun, 3 Dec 2023 21:29:38 +0800
>>> From: Zhao Liu <[email protected]>
>>> Subject: [PATCH v3 0/9] drm/i915: Replace kmap_atomic() with
>>> kmap_local_page()
>>> X-Mailer: git-send-email 2.34.1
>>>
>>> From: Zhao Liu <[email protected]>
>>>
>>> Hi all,
>>>
>>> I refreshed this v3 by rebasing v2 [1] on the commit 968f35f4ab1c
>>> ("Merge tag 'v6.7-rc3-smb3-client-fixes' of git://git.samba.org/sfrench/
>>> cifs-2.6").
>>>
>>> Based on the current code, I rechecked the substitutions in v2 and they
>>> still stand and are valid, so no code change in v3.
>>>
>>> Thanks for all the review! And sorry v2 was missed, I'll pay more
>>> attention to this v3.
>>>
>>>
>>> Purpose of This Patchset
>>> ========================
>>>
>>> The purpose of this pacthset is to replace all uses of kmap_atomic() in
>>> i915 with kmap_local_page() because the use of kmap_atomic() is being
>>> deprecated in favor of kmap_local_page()[2]. And 92b64bd (mm/highmem:
>>> add notes about conversions from kmap{,_atomic}()) has declared the
>>> deprecation of kmap_atomic().
>>>
>>>
>>> Motivation for Deprecating kmap_atomic() and Using kmap_local_page()
>>> ====================================================================
>>>
>>> The main difference between atomic and local mappings is that local
>>> mappings doesn't disable page faults or preemption (the preemption is
>>> disabled for !PREEMPT_RT case, otherwise it only disables migration).
>>>
>>> With kmap_local_page(), we can avoid the often unwanted side effect of
>>> unnecessary page faults and preemption disables.
>>>
>>>
>>> Patch summary
>>> =============
>>>
>>> Patch 1, 4-6 and 8-9 replace kmap_atomic()/kunmap_atomic() with
>>> kmap_local_page()/kunmap_local() directly. With these local
>>> mappings, the page faults and preemption are allowed.
>>>
>>> Patch 2 and 7 use memcpy_from_page() and memcpy_to_page() to replace
>>> kmap_atomic()/kunmap_atomic(). These two variants of memcpy()
>>> are based on the local mapping, so page faults and preemption
>>> are also allowed in these two interfaces.
>>>
>>> Patch 3 replaces kmap_atomic()/kunmap_atomic() with kmap_local_page()/
>>> kunmap_local() and also disable page fault since the for
>>> special
>>> handling (pls see the commit message).
>>>
>>>
>>> Reference
>>> =========
>>>
>>> [1]:
>>> https://lore.kernel.org/all/[email protected]/
>>> [2]:
>>> https://lore.kernel.org/all/[email protected]
>>>
>>>
>>> Thanks and Best Regards,
>>> Zhao
>>>
>>> ---
>>> Changlog:
>>>
>>> Changes since v2:
>>> * Rebased on 968f35f4ab1c ("Merge tag 'v6.7-rc3-smb3-client-fixes' of
>>> git://git.samba.org/sfrench/cifs-2.6").
>>> * Removed changelog (of v2) in commit message.
>>> * Fixed typo in cover letter (Fabio).
>>> * Added Reviewed-by tags from Ira and Fabio.
>>>
>>> Changes since v1:
>>> * Dropped hot plug related description in commit message since it has
>>> nothing to do with kmap_local_page().
>>> * Emphasized the motivation for using kmap_local_page() in commit
>>> message.
>>> * Rebased patch 1 on f47e630 (drm/i915/gem: Typecheck page lookups) to
>>> keep the "idx" variable of type pgoff_t here.
>>> * Used memcpy_from_page() and memcpy_to_page() to replace
>>> kmap_local_page() + memcpy() in patch 2.
>>>
>>> ---
>>> Zhao Liu (9):
>>> drm/i915: Use kmap_local_page() in gem/i915_gem_object.c
>>> drm/i915: Use memcpy_[from/to]_page() in gem/i915_gem_pyhs.c
>>> drm/i915: Use kmap_local_page() in gem/i915_gem_shmem.c
>>> drm/i915: Use kmap_local_page() in gem/selftests/huge_pages.c
>>> drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_coherency.c
>>> drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_context.c
>>> drm/i915: Use memcpy_from_page() in gt/uc/intel_uc_fw.c
>>> drm/i915: Use kmap_local_page() in i915_cmd_parser.c
>>> drm/i915: Use kmap_local_page() in gem/i915_gem_execbuffer.c
>>>
>>> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +++++-----
>>> drivers/gpu/drm/i915/gem/i915_gem_object.c | 8 +++-----
>>> drivers/gpu/drm/i915/gem/i915_gem_phys.c | 10 ++--------
>>> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 ++++--
>>> drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 6 +++---
>>> .../gpu/drm/i915/gem/selftests/i915_gem_coherency.c | 12 ++++--------
>>> .../gpu/drm/i915/gem/selftests/i915_gem_context.c | 8 ++++----
>>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 +----
>>> drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++--
>>> 9 files changed, 28 insertions(+), 41 deletions(-)
>>>
>>> --
>>> 2.34.1
>>>
On Thu, Dec 14, 2023 at 02:35:26PM +0000, Tvrtko Ursulin wrote:
> Date: Thu, 14 Dec 2023 14:35:26 +0000
> From: Tvrtko Ursulin <[email protected]>
> Subject: Re: [PATCH v3 0/9] drm/i915: Replace kmap_atomic() with
> kmap_local_page()
>
>
> On 14/12/2023 13:45, Tvrtko Ursulin wrote:
> >
> > Hi Zhao,
> >
> > On 14/12/2023 13:19, Zhao Liu wrote:
> > > Hi maintainers,
> > >
> > > Just kindly ping.
> > > May I ask if this refresh version could be merged into the next tree of
> > > the i915?
> >
> > I certainly spotted your series last week or so but then it slipped my
> > mind to go through it. Should be able to go through it today or
> > tomorrow.
>
> It all looks good to me. I only needed to queue a re-test in our CI since v3
> failed BAT, but pretty sure it wasn't at fault. Once I am satisfied with the
> results I will merge the series. Thanks for the cleanups and your patience!
>
> Regards,
>
> Tvrtko
>
Thanks for your review!
Regards,
Zhao
>
> > Regards,
> >
> > Tvrtko
> >
> > >
> > > Thanks,
> > > Zhao
> > >
> > > On Sun, Dec 03, 2023 at 09:29:38PM +0800, Zhao Liu wrote:
> > > > Date: Sun, 3 Dec 2023 21:29:38 +0800
> > > > From: Zhao Liu <[email protected]>
> > > > Subject: [PATCH v3 0/9] drm/i915: Replace kmap_atomic() with
> > > > ? kmap_local_page()
> > > > X-Mailer: git-send-email 2.34.1
> > > >
> > > > From: Zhao Liu <[email protected]>
> > > >
> > > > Hi all,
> > > >
> > > > I refreshed this v3 by rebasing v2 [1] on the commit 968f35f4ab1c
> > > > ("Merge tag 'v6.7-rc3-smb3-client-fixes' of git://git.samba.org/sfrench/
> > > > cifs-2.6").
> > > >
> > > > Based on the current code, I rechecked the substitutions in v2 and they
> > > > still stand and are valid, so no code change in v3.
> > > >
> > > > Thanks for all the review! And sorry v2 was missed, I'll pay more
> > > > attention to this v3.
> > > >
> > > >
> > > > Purpose of This Patchset
> > > > ========================
> > > >
> > > > The purpose of this pacthset is to replace all uses of kmap_atomic() in
> > > > i915 with kmap_local_page() because the use of kmap_atomic() is being
> > > > deprecated in favor of kmap_local_page()[2]. And 92b64bd (mm/highmem:
> > > > add notes about conversions from kmap{,_atomic}()) has declared the
> > > > deprecation of kmap_atomic().
> > > >
> > > >
> > > > Motivation for Deprecating kmap_atomic() and Using kmap_local_page()
> > > > ====================================================================
> > > >
> > > > The main difference between atomic and local mappings is that local
> > > > mappings doesn't disable page faults or preemption (the preemption is
> > > > disabled for !PREEMPT_RT case, otherwise it only disables migration).
> > > >
> > > > With kmap_local_page(), we can avoid the often unwanted side effect of
> > > > unnecessary page faults and preemption disables.
> > > >
> > > >
> > > > Patch summary
> > > > =============
> > > >
> > > > Patch 1, 4-6 and 8-9 replace kmap_atomic()/kunmap_atomic() with
> > > > ???????? kmap_local_page()/kunmap_local() directly. With these local
> > > > ???????? mappings, the page faults and preemption are allowed.
> > > >
> > > > Patch 2 and 7 use memcpy_from_page() and memcpy_to_page() to replace
> > > > ???????? kmap_atomic()/kunmap_atomic(). These two variants of memcpy()
> > > > ???????? are based on the local mapping, so page faults and preemption
> > > > ???????? are also allowed in these two interfaces.
> > > >
> > > > Patch 3 replaces kmap_atomic()/kunmap_atomic() with kmap_local_page()/
> > > > ???????? kunmap_local() and also disable page fault since the
> > > > for special
> > > > ???????? handling (pls see the commit message).
> > > >
> > > >
> > > > Reference
> > > > =========
> > > >
> > > > [1]: https://lore.kernel.org/all/[email protected]/
> > > > [2]:
> > > > https://lore.kernel.org/all/[email protected]
> > > >
> > > >
> > > > Thanks and Best Regards,
> > > > Zhao
> > > >
> > > > ---
> > > > Changlog:
> > > >
> > > > Changes since v2:
> > > > * Rebased on 968f35f4ab1c ("Merge tag 'v6.7-rc3-smb3-client-fixes' of
> > > > ?? git://git.samba.org/sfrench/cifs-2.6").
> > > > * Removed changelog (of v2) in commit message.
> > > > * Fixed typo in cover letter (Fabio).
> > > > * Added Reviewed-by tags from Ira and Fabio.
> > > >
> > > > Changes since v1:
> > > > * Dropped hot plug related description in commit message since it has
> > > > ?? nothing to do with kmap_local_page().
> > > > * Emphasized the motivation for using kmap_local_page() in commit
> > > > ?? message.
> > > > * Rebased patch 1 on f47e630 (drm/i915/gem: Typecheck page lookups) to
> > > > ?? keep the "idx" variable of type pgoff_t here.
> > > > * Used memcpy_from_page() and memcpy_to_page() to replace
> > > > ?? kmap_local_page() + memcpy() in patch 2.
> > > >
> > > > ---
> > > > Zhao Liu (9):
> > > > ?? drm/i915: Use kmap_local_page() in gem/i915_gem_object.c
> > > > ?? drm/i915: Use memcpy_[from/to]_page() in gem/i915_gem_pyhs.c
> > > > ?? drm/i915: Use kmap_local_page() in gem/i915_gem_shmem.c
> > > > ?? drm/i915: Use kmap_local_page() in gem/selftests/huge_pages.c
> > > > ?? drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_coherency.c
> > > > ?? drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_context.c
> > > > ?? drm/i915: Use memcpy_from_page() in gt/uc/intel_uc_fw.c
> > > > ?? drm/i915: Use kmap_local_page() in i915_cmd_parser.c
> > > > ?? drm/i915: Use kmap_local_page() in gem/i915_gem_execbuffer.c
> > > >
> > > > ? drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c?????? | 10 +++++-----
> > > > ? drivers/gpu/drm/i915/gem/i915_gem_object.c?????????? |? 8 +++-----
> > > > ? drivers/gpu/drm/i915/gem/i915_gem_phys.c???????????? | 10 ++--------
> > > > ? drivers/gpu/drm/i915/gem/i915_gem_shmem.c??????????? |? 6 ++++--
> > > > ? drivers/gpu/drm/i915/gem/selftests/huge_pages.c????? |? 6 +++---
> > > > ? .../gpu/drm/i915/gem/selftests/i915_gem_coherency.c? | 12 ++++--------
> > > > ? .../gpu/drm/i915/gem/selftests/i915_gem_context.c??? |? 8 ++++----
> > > > ? drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c???????????? |? 5 +----
> > > > ? drivers/gpu/drm/i915/i915_cmd_parser.c?????????????? |? 4 ++--
> > > > ? 9 files changed, 28 insertions(+), 41 deletions(-)
> > > >
> > > > --
> > > > 2.34.1
> > > >
On 14/12/2023 15:04, Zhao Liu wrote:
> On Thu, Dec 14, 2023 at 02:35:26PM +0000, Tvrtko Ursulin wrote:
>> Date: Thu, 14 Dec 2023 14:35:26 +0000
>> From: Tvrtko Ursulin <[email protected]>
>> Subject: Re: [PATCH v3 0/9] drm/i915: Replace kmap_atomic() with
>> kmap_local_page()
>>
>>
>> On 14/12/2023 13:45, Tvrtko Ursulin wrote:
>>>
>>> Hi Zhao,
>>>
>>> On 14/12/2023 13:19, Zhao Liu wrote:
>>>> Hi maintainers,
>>>>
>>>> Just kindly ping.
>>>> May I ask if this refresh version could be merged into the next tree of
>>>> the i915?
>>>
>>> I certainly spotted your series last week or so but then it slipped my
>>> mind to go through it. Should be able to go through it today or
>>> tomorrow.
>>
>> It all looks good to me. I only needed to queue a re-test in our CI since v3
>> failed BAT, but pretty sure it wasn't at fault. Once I am satisfied with the
>> results I will merge the series. Thanks for the cleanups and your patience!
>>
>> Regards,
>>
>> Tvrtko
>>
>
> Thanks for your review!
Pushed to drm-intel-gt-next, thanks again!
Regards,
Tvrtko