Re: [PATCH v2 3/8] vringh: replace kmap_atomic() with kmap_local_page()

On giovedì 16 marzo 2023 09:09:29 CET Stefano Garzarella wrote:

On Wed, Mar 15, 2023 at 10:12 PM Fabio M. De Francesco

[off-list ref] wrote:

quoted

On martedì 14 marzo 2023 04:56:08 CET Jason Wang wrote:

quoted

On Thu, Mar 2, 2023 at 7:34 PM Stefano Garzarella [off-list ref]

wrote:

quoted

quoted

kmap_atomic() is deprecated in favor of kmap_local_page().

It's better to mention the commit or code that introduces this.

quoted

With kmap_local_page() the mappings are per thread, CPU local, can 

take

quoted

quoted

quoted

page-faults, and can be called from any context (including 

interrupts).

quoted

quoted

quoted

Furthermore, the tasks can be preempted and, when they are scheduled 

to

quoted

quoted

quoted

run again, the kernel virtual addresses are restored and still valid.

kmap_atomic() is implemented like a kmap_local_page() which also
disables
page-faults and preemption (the latter only for !PREEMPT_RT kernels,
otherwise it only disables migration).

The code within the mappings/un-mappings in getu16_iotlb() and
putu16_iotlb() don't depend on the above-mentioned side effects of
kmap_atomic(),

Note we used to use spinlock to protect simulators (at least until
patch 7, so we probably need to re-order the patches at least) so I
think this is only valid when:

The vringh IOTLB helpers are not used in atomic context (e.g spinlock,
interrupts).

I'm probably missing some context but it looks that you are saying that
kmap_local_page() is not suited for any use in atomic context (you are
mentioning spinlocks).

The commit message (that I know pretty well since it's the exact copy, 

word

quoted

by word, of my boiler plate commits)

I hope it's not a problem for you, should I mention it somehow?

Sorry, I had missed your last message when I wrote a another message few 
minutes ago in this thread.

Obviously, I'm happy that my commit message it's being reused. As I said in 
the other message I would appreciate some kind of crediting me as the author.

I proposed a means you can use, but feel free to ignore my suggestion and do 
differently if you prefer to.

Again thanks,

Fabio

I searched for the last commits that made a similar change and found
yours that explained it perfectly ;-)

Do I need to rephrase?

quoted

explains that kmap_local_page() is perfectly
usable in atomic context (including interrupts).

I don't know this code, however I am not able to see why these vringh 

IOTLB

quoted

helpers cannot work if used under spinlocks. Can you please elaborate a
little more?

quoted

If yes, should we document this? (Or should we introduce a boolean to
say whether an IOTLB variant can be used in an atomic context)?

Again, you'll have no problems from the use of kmap_local_page() and so 

you

quoted

don't need any boolean to tell whether or not the code is running in 

atomic

quoted

context.

Please take a look at the Highmem documentation which has been recently
reworked and extended by me: https://docs.kernel.org/mm/highmem.html

Anyway, I have been ATK 12 or 13 hours in a row. So I'm probably missing 

the

quoted

whole picture.

Thanks for your useful info!
Stefano