On martedì 6 dicembre 2022 09:00:10 CET Sebastian Andrzej Siewior wrote: > On 2022-12-06 08:00:29 [+0100], Fabio M. De Francesco wrote: > > diff --git a/Documentation/mm/highmem.rst b/Documentation/mm/highmem.rst > > index 0f731d9196b0..9523e92299f6 100644 > > --- a/Documentation/mm/highmem.rst > > +++ b/Documentation/mm/highmem.rst > > @@ -100,10 +101,21 @@ list shows them in order of preference of use. > > > > (included in the "Functions" section) for details on how to manage nested > > mappings. > > > > -* kmap_atomic(). This permits a very short duration mapping of a single > > - page. Since the mapping is restricted to the CPU that issued it, it > > - performs well, but the issuing task is therefore required to stay on that > > - CPU until it has finished, lest some other task displace its mappings. > > +* kmap_atomic(). This function has been deprecated; use kmap_local_page(). > > + > > + NOTE: Conversions to kmap_local_page() must take care to follow the > > mapping + restrictions imposed on kmap_local_page(). Furthermore, code > > between the + map/unmap operations may implicitly depended on the side > > effects of + kmap_atomic(), such as disabling pagefaults, migration, > > and/or preemption. + Such conversions should be changed to make explicit > > calls for those + requirements. Sebastian, thanks for taking a look at my patch and replying. > Furthermore, code between the kmap_atomic() and kunmap_atomic() > functions may implicitly depended I suppose it should be "depend"? Shouldn't it? > on the side effects of kmap_atomic() > namely disabling pagefaults or preemption or both. I agree with you for rephrasing, mainly because it is written in poor English. However, I still have doubts about why you deleted "migration". AFAIK, __kmap_local_pfn_prot() always takes care of disabling migration for HIGHMEM enabled kernels. How about !HIGHMEM, where kmap_local_page() is an indirect call to page_address()? Did you mean that, if the code between kmap_atomic() and kunmap_atomic() depended on migrate_disable() (in PREEMPT_RT) we should always just stay safe and call preempt_disable() together with conversion to kmap_local_page()? If so, I understand and I again agree with you. If not, I'm missing something; so please let me understand properly. Aside from the above, I'm not sure whether you deleted the last phrase before your suggestion. What about making it to become "For the above-mentioned cases, conversions should also explicitly disable page-faults and/or preemption"? Thanks again for noticing my mistakes. Fabio > > > + [Legacy documentation] > > + > > + This permits a very short duration mapping of a single page. Since the > > + mapping is restricted to the CPU that issued it, it performs well, but > > + the issuing task is therefore required to stay on that CPU until it has > > + finished, lest some other task displace its mappings. > > > > kmap_atomic() may also be used by interrupt contexts, since it does not > > sleep and the callers too may not sleep until after kunmap_atomic() is > > Sebastian
