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

["Fabio M. De Francesco" <fmdefrancesco@gmail.com>]

On mercoledì 21 giugno 2023 11:06:51 CEST Thomas Hellström (Intel) wrote:
> 
> I think one thing worth mentioning in the context of this patch is that
> IIRC kmap_local_page() will block offlining of the mapping CPU until
> kunmap_local(),

Migration is disabled.

> so while I haven't seen any guidelines around the usage
> of this api for long-held mappings,

It would be advisable to not use it for long term mappings, if possible. These 
"local" mappings should better be help for not too long duration. 

> I figure it's wise to keep the
> mapping duration short, or at least avoid sleeping with a
> kmap_local_page() map active.

Nothing prevents a call of preempt_disable() around the section of code 
between kmap_local_page() / kunmap_local(). If it is needed, local mappings 
could also be acquired under spinlocks and/or with disabled interrupts.

I don't know the code, however, everything cited above could be the subject of 
a subsequent patch.

Regards,

Fabio

> I figured, while page compression is probably to be considered "slow"
> it's probably not slow enough to motivate kmap() instead of
> kmap_local_page(), but if anyone feels differently, perhaps it should be
> considered.
> 
> With that said, my Reviewed-by: still stands.
> 
> /Thomas
>