lkml.org 
[lkml]   [2023]   [Mar]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v2 6/9] drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_context.c
    Date
    From: Zhao Liu <zhao1.liu@intel.com>

    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/20220813220034.806698-1-ira.weiny@intel.com

    v2:
    * Dropped hot plug related description since it has nothing to do with
    kmap_local_page().
    * No code change since v1, and added description of the motivation of
    using kmap_local_page().

    Suggested-by: Dave Hansen <dave.hansen@intel.com>
    Suggested-by: Ira Weiny <ira.weiny@intel.com>
    Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
    Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
    ---
    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 a81fa6a20f5a..dcbc0b8e3323 100644
    --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
    +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
    @@ -481,12 +481,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);
    @@ -512,7 +512,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);

    @@ -538,7 +538,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
    \
     
     \ /
      Last update: 2023-03-29 09:26    [W:4.185 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site