Re: [PATCH] x86/sgx: Replace kmap/kunmap_atomic calls

From: Fabio M. De Francesco
Date: Thu Oct 06 2022 - 18:03:12 EST

On Thursday, October 6, 2022 10:45:56 PM CEST Dave Hansen wrote:
> On 10/6/22 13:37, Fabio M. De Francesco wrote:
> > kmap() were not suited in those cases because it might sleep. If the
> > of the author are simply map a page while in atomic, so to avoid sleeping
> > atomic bugs, your conversions looks good.
> >
> > For the reasons above, can you please say something more about why this
> > needed a kmap_atomic() instead of calling kmap()?
> This question is backwards. kmap_atomic() is the default that folks
> use.

Sorry, I can't understand why kmap_atomic() is the default. What advantage we
get from disabling pagefaults and probably also preemption (it depends on !
PREEMPT_RT also when we don't need that the kernel runs in atomic?

Do you mean that the more code run in atomic, the less pagefaults we allow,
the less preemption we allow, and so on, the better we get from Linux?

Do you remember that what I say above happens both on 64 bits systems as well
as in 32 bits?

I'm a newcomer so I may be wrong, however I think that in 64 bits systems we
gain from simply returning page_address() and from finer granularity
(less atomic, less critical sections, instead more preemption and / or

Why shouldn't be kmap() the default choice in a preemptible kernel if sections
don't need that disabling of pagefaults, along with preemption (which get
disabled many more times that only migration)?

Am I still missing anything fundamental?

> You use kmap_atomic() *always* unless you _need_ to sleep or one
> of the other kmap()-only things.

What would happen if you rely on switching in atomic as a side effect of
kmap_atomic() and then you convert to kmap_local_page() without explicitly
disabling, for example, preemption since who converts don't care to know if
the code is in atomic before calling kmap_atomic() before or after the call
(as I said there may be cases where non atomic execution must disable
preemption for some reasons only between the mapping and the unmapping?

If I were a maintainer I wouldn't trust changes that let me think that the
developer can't tell if we need to disable something while converting to

I hope this time I've been to convey more clearly my thoughts. I'm sorry for
my scarce knowledge of English.



> Folks don't and shouldn't have to explain why this was using kmap_atomic().