[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6583463e8de5c_30a94294fb@iweiny-mobl.notmuch>
Date: Wed, 20 Dec 2023 11:53:34 -0800
From: Ira Weiny <ira.weiny@...el.com>
To: "Fabio M. De Francesco" <fabio.maria.de.francesco@...ux.intel.com>,
"Andrew Morton" <akpm@...ux-foundation.org>, <linux-mm@...ck.org>,
<linux-kernel@...r.kernel.org>, Ira Weiny <ira.weiny@...el.com>
CC: Ira Weiny <ira.weiny@...el.com>
Subject: Re: [PATCH] mm/memory: Replace kmap() with kmap_local_page()
Fabio M. De Francesco wrote:
> On Monday, 18 December 2023 04:34:13 CET Ira Weiny wrote:
> > Fabio M. De Francesco wrote:
> >
> > [snip]
> >
> > > Cc: Ira Weiny <ira.weiny@...el.com>
> > > Signed-off-by: Fabio M. De Francesco
> > > <fabio.maria.de.francesco@...ux.intel.com> ---
> > >
> > > mm/memory.c | 5 ++---
> > > 1 file changed, 2 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/mm/memory.c b/mm/memory.c
> > > index 7d9f6b685032..88377a107fbe 100644
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -5852,7 +5852,7 @@ static int __access_remote_vm(struct mm_struct *mm,
> > > unsigned long addr,>
> > > if (bytes > PAGE_SIZE-offset)
> > >
> > > bytes = PAGE_SIZE-offset;
> > >
> > > - maddr = kmap(page);
> > > + maddr = kmap_local_page(page);
> > >
> > > if (write) {
> > >
> > > copy_to_user_page(vma, page, addr,
> > >
> > > maddr + offset, buf,
> bytes);
> > >
> > > @@ -5861,8 +5861,7 @@ static int __access_remote_vm(struct mm_struct *mm,
> > > unsigned long addr,>
> > > copy_from_user_page(vma, page, addr,
> > >
> > > buf, maddr + offset,
> bytes);
> > >
> > > }
> > >
> > > - kunmap(page);
> > > - put_page(page);
> > > + unmap_and_put_page(page, maddr);
> >
> > Does this really have the same functionality?
> >
> > Ira
>
> Do you have any specific reasons to say that?
>
> The unmap_and_put_page() helper was created by Al Viro (it initially was
> put_and_unmap_page() and I sent a patch to rename it to the current name). He
> noticed that we have lots of kunmap_local() followed by put_page().
>
> The current implementation has then been changed (Matthew did it, if I
> remember correctly).
>
> My understanding of the current implementation is that unmap_and_put_page()
> calls folio_release_kmap(), taking as arguments the folio which the page
> belongs to and the kernel virtual address returned by kmap_local_page().
>
> folio_release_kmap() calls kunmap_local() and then folio_put(). The last is
> called on the folio obtained by the unmap_and_put_page() wrapper and, if I'm
> not wrong, it releases refcounts on folios like put_page() does on pages.
This is where my consternation came from. I saw the folio_put() and did
not realize that get_page() now calls folio_get().
>
> Am I missing something?
Nope, I just did not have time to trace code yesterday.
Reviewed-by: Ira Weiny <ira.weiny@...el.com>
Powered by blists - more mailing lists