Received-SPF: pass (google.com: domain of linux-kernel+bounces-3119-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: "Fabio M. De Francesco" <fabio.maria.de.francesco@linux.intel.com>
To: Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org,
 linux-kernel@vger.kernel.org, Ira Weiny <ira.weiny@intel.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Subject: Re: [PATCH] mm/memory: Replace kmap() with kmap_local_page()
Date: Mon, 18 Dec 2023 08:43:11 +0100
Message-ID: <4255260.irdbgypaU6@fdefranc-mobl3>
Organization: intel
In-Reply-To: <657fbdb5db945_126a129483@iweiny-mobl.notmuch>
References:
 <20231215084417.2002370-1-fabio.maria.de.francesco@linux.intel.com>
 <657fbdb5db945_126a129483@iweiny-mobl.notmuch>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"

On Monday, 18 December 2023 04:34:13 CET Ira Weiny wrote:
> Fabio M. De Francesco wrote:
> 
> [snip]
> 
> > Cc: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Fabio M. De Francesco
> > <fabio.maria.de.francesco@linux.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.

Am I missing something?

For further reference, please take a look at the following path from Al Viro 
that is modelled after my conversions in fs/sysv: https://lore.kernel.org/all/
20231213000849.2748576-4-viro@zeniv.linux.org.uk/

Thanks,

Fabio