On sabato 17 giugno 2023 20:04:20 CEST Sumitra Sharma wrote: > kmap() has been deprecated in favor of the kmap_local_page() > due to high cost, restricted mapping space, the overhead of a > global lock for synchronization, and making the process sleep > in the absence of free slots. > > kmap_local_page() is faster than kmap() and offers thread-local > and CPU-local mappings, take pagefaults in a local kmap region NIT: _can_ take pagefaults in a local kmap region > and preserves preemption by saving the mappings of outgoing tasks > and restoring those of the incoming one during a context switch. > > The mapping is kept thread local in the function > “i915_vma_coredump_create” in i915_gpu_error.c > > Therefore, replace kmap() with kmap_local_page(). > > Suggested-by: Ira Weiny <ira.weiny@xxxxxxxxx> > > Signed-off-by: Sumitra Sharma <sumitraartsy@xxxxxxxxx> > --- > > Changes in v2: > - Replace kmap() with kmap_local_page(). > - Change commit subject and message. With the changes that Ira suggested and the minor fix I'm proposing to the commit message, it looks good to me too, so this patch is... Reviewed-by: Fabio M. De Francesco <fmdefrancesco@xxxxxxxxx> However, as far as I'm concerned, our nits don't necessarily require any newer version, especially because Tvrtko has already sent this patch for their CI. Thanks, Fabio P.S.: As Sumitra says both kmap() and kmap_local_page() allows preemption in non atomic context. Furthermore, Tvrtko confirmed that the pages can come from HIGHMEM, therefore kmap_local_page for local temporary mapping is unavoidable. Last thing... Thomas thinks he wants to make it run atomically (if I understood one of his messages correctly). As I already responded, nothing prevents someone does another patch just to disable preemption (or to enter atomic context by other means) around the code marked by kmap_local_page() / kunmap_local() because these functions work perfectly _also_ in atomic context (including interrupts). But this is not something that Sumitra should be worried about. > > drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c > b/drivers/gpu/drm/i915/i915_gpu_error.c index f020c0086fbc..bc41500eedf5 > 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -1164,9 +1164,9 @@ i915_vma_coredump_create(const struct intel_gt *gt, > > drm_clflush_pages(&page, 1); > > - s = kmap(page); > + s = kmap_local_page(page); > ret = compress_page(compress, s, dst, false); > - kunmap(page); > + kunmap_local(s); > > drm_clflush_pages(&page, 1); > > -- > 2.25.1
