Re: [PATCH v2] drm/i915: Replace kmap() with kmap_local_page()

[Tvrtko Ursulin]

On 19/06/2023 08:59, Thomas Hellström (Intel) wrote:
> On 6/18/23 20:11, Ira Weiny wrote:
> > 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
> > > 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>
> > NIT: No need for the line break between Suggested-by and your signed 
> > off line.
> >
> > > Signed-off-by: Sumitra Sharma <sumitraartsy@xxxxxxxxx>
> > > ---
> > >
> > > Changes in v2:
> > >     - Replace kmap() with kmap_local_page().
> > Generally it is customary to attribute a change like this to those who
> > suggested it in a V1 review.
> >
> > For example:
> >
> >       - Tvrtko/Thomas: Use kmap_local_page() instead of page_address()
> >
> > Also I don't see Thomas on the new email list.  Since he took the time to
> > review V1 he might want to check this version out.  I've added him to the
> > 'To:' list.
>
> Thanks.
>
>
> > Also a link to V1 is nice.  B4 formats it like this:
> >
> > - Link to v1: 
> > https://lore.kernel.org/all/20230614123556.GA381200@xxxxxxxxxxx/
> >
> > All that said the code looks good to me.  So with the above changes.
> >
> > Reviewed-by: Ira Weiny <ira.weiny@xxxxxxxxx>
>
> LGTM. Reviewed-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>

Thanks all! I'll just re-send the patch for our CI, since it didn't get 
picked up automatically (stuck in moderation perhaps), with all r-b tags 
added and extra line space removed and merge it if results will be green.

Regards,

Tvrtko