lkml.org 
[lkml]   [2022]   [Aug]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v2] kexec: Replace kmap() with kmap_local_page()
    Date
    On martedì 2 agosto 2022 05:06:54 CEST Baoquan He wrote:
    > On 07/08/22 at 01:15am, Fabio M. De Francesco wrote:
    > > The use of kmap() and kmap_atomic() are being deprecated in favor of
    > > kmap_local_page().
    > >
    > > With kmap_local_page(), the mappings are per thread, CPU local and not
    > > globally visible. Furthermore, the mappings can be acquired from any
    > > context (including interrupts).
    > >
    > > Therefore, use kmap_local_page() in kexec_core.c because these mappings
    are
    > > per thread, CPU local, and not globally visible.
    > >
    > > Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
    > > enabled.
    >
    > Wondering what arch you tested with.

    I'm sorry, I forgot to say that I use x86_32 with 4GB to 6GB RAM.
    This is usually an information that I add in the commit messages of all the
    recent conversions I'm working on across the entire kernel.

    Another important information (overlooked again this time) is that (1)
    kmap() comes with an overhead as mapping space is restricted and protected
    by a global lock for synchronization and (2) it also requires global TLB
    invalidation when the kmap’s pool wraps and it might block when the mapping
    space is fully utilized until a slot becomes available.

    More information about why these kmap() to kmap_local_page() conversions
    are needed / preferred can be found in the recent changes I made to
    highmem.rst. They are already in mainline since about two months.

    A second round of additional changes has been taken by Andrew M. just few
    days ago.

    My goal is to convert the most of the kmap() call sites that are still left
    across the whole kernel. I'm not yet sure that these kinds of conversions
    can be done everywhere, especially if the kernel virtual address of the
    mapping is handed to other contexts, because this would invalidate the
    pointer returned by kmap_local_page().

    > This looks good, but may not benefit much. Say so because I doubt
    > how many 32bit systems are using kexec/kdump mechanism.

    I really cannot say nothing about how many 32 bits systems are using kexec/
    kdump mechanism, however I still think that the conversions are worth
    everywhere.

    > Anyway,
    >
    > Acked-by: Baoquan He <bhe@redhat.com>
    >

    Thank you so much!

    Fabio

    > >
    > > Suggested-by: Ira Weiny <ira.weiny@intel.com>
    > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
    > > ---
    > >
    > > v1->v2: A sentence of the commit message contained an error due to a
    > > mistake in copy-pasting from a previous patch. Replace "aio.c" with
    > > "kexec_core.c".
    > >
    > > kernel/kexec_core.c | 8 ++++----
    > > 1 file changed, 4 insertions(+), 4 deletions(-)
    > >
    > > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
    > > index 4d34c78334ce..6f98274765d4 100644
    > > --- a/kernel/kexec_core.c
    > > +++ b/kernel/kexec_core.c
    > > @@ -814,7 +814,7 @@ static int kimage_load_normal_segment(struct kimage
    *image,
    > > if (result < 0)
    > > goto out;
    > >
    > > - ptr = kmap(page);
    > > + ptr = kmap_local_page(page);
    > > /* Start with a clear page */
    > > clear_page(ptr);
    > > ptr += maddr & ~PAGE_MASK;
    > > @@ -827,7 +827,7 @@ static int kimage_load_normal_segment(struct kimage
    *image,
    > > memcpy(ptr, kbuf, uchunk);
    > > else
    > > result = copy_from_user(ptr, buf, uchunk);
    > > - kunmap(page);
    > > + kunmap_local(ptr);
    > > if (result) {
    > > result = -EFAULT;
    > > goto out;
    > > @@ -878,7 +878,7 @@ static int kimage_load_crash_segment(struct kimage
    *image,
    > > goto out;
    > > }
    > > arch_kexec_post_alloc_pages(page_address(page), 1, 0);
    > > - ptr = kmap(page);
    > > + ptr = kmap_local_page(page);
    > > ptr += maddr & ~PAGE_MASK;
    > > mchunk = min_t(size_t, mbytes,
    > > PAGE_SIZE - (maddr & ~PAGE_MASK));
    > > @@ -894,7 +894,7 @@ static int kimage_load_crash_segment(struct kimage
    *image,
    > > else
    > > result = copy_from_user(ptr, buf, uchunk);
    > > kexec_flush_icache_page(page);
    > > - kunmap(page);
    > > + kunmap_local(ptr);
    > > arch_kexec_pre_free_pages(page_address(page), 1);
    > > if (result) {
    > > result = -EFAULT;
    > > --
    > > 2.36.1
    > >
    > >
    > > _______________________________________________
    > > kexec mailing list
    > > kexec@lists.infradead.org
    > > http://lists.infradead.org/mailman/listinfo/kexec
    > >
    >
    >




    \
     
     \ /
      Last update: 2022-08-02 08:21    [W:18.661 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site