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 intents > > of the author are simply map a page while in atomic, so to avoid sleeping in > > atomic bugs, your conversions looks good. > > > > For the reasons above, can you please say something more about why this code > > 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 migration). 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 kmap_local_page(). I hope this time I've been to convey more clearly my thoughts. I'm sorry for my scarce knowledge of English. Thanks, Fabio > > Folks don't and shouldn't have to explain why this was using kmap_atomic(). >
