On 20/06/2023 14:23, Ira Weiny wrote:
Sumitra Sharma wrote:
On Sun, Jun 18, 2023 at 11:11:08AM -0700, 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.we...@intel.com>


NIT: No need for the line break between Suggested-by and your signed off line.


Hi Ira,

What does NIT stand for?

Shorthand for 'nitpicking'.

"giving too much attention to details that are not important, especially
as a way of criticizing: "

        - https://dictionary.cambridge.org/dictionary/english/nitpicking

Via email this is a way for authors of an email to indicate something is
technically wrong but while nicely acknowledging that it is not very
significant and could be seen as overly critical.

For this particular comment I'm showing something to pay attention to next
time but that was not a big deal this time around.


Thank you. I will take care about the line breaks.

Signed-off-by: Sumitra Sharma <sumitraar...@gmail.com>
---

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.

Also a link to V1 is nice.  B4 formats it like this:

- Link to v1: https://lore.kernel.org/all/20230614123556.ga381...@sumitra.com/

All that said the code looks good to me.  So with the above changes.

Reviewed-by: Ira Weiny <ira.we...@intel.com>


I have noted down the points mentioned above. Thank you again.

I am not supposed to create another version of this patch for
adding the above mentions, as you and Thomas both gave this patch
a reviewed-by tag. Right?


Based on this response[*] from Tvrtko I think this version can move
through without a v3.

Thanks!
Ira

[*] 
https://lore.kernel.org/all/bcb0a1d2-cd4d-a56f-1ee6-7ccfdd2f7...@linux.intel.com/

<quote>
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
</quote>

Pushed to drm-intel-gt-next, thanks for the patch and reviews!

Regards,

Tvrtko

Reply via email to