Re: Should we delete memmove_page?

[Ira Weiny <ira.weiny@intel.com>]

On Fri, May 27, 2022 at 02:26:52AM +0100, Matthew Wilcox wrote:
> On Thu, May 26, 2022 at 05:40:44PM -0700, Ira Weiny wrote:
> > On Thu, May 26, 2022 at 03:48:28PM +0100, Matthew Wilcox wrote:
> > > 
> > > I was looking at memmove_page() and it occurred to me that it can't
> > > actually work,
> > 
> > Oh wow yea.  Because you can't unmap that address correctly.
> 
> I don't understand what you mean ...

Nevermind.  I missed the fact that arch_kmap_local_map_idx() does not actually
use the pfn except on xtensa.

> you can unmap the address, it's
> just that memmove can't know that the two virtual ranges actually
> overlap physically.

I was thinking that the index returned would be the same for both kmaps.

> 
> > Yes deletion is best.  But...
> > 
> > 	copy_user_highpage()
> > 	copy_highpage()
> > 
> > ... might suffer from the same potential issue should a user not realize.  I
> > think memcpy_page() by virtue of the name.
> 
> Umm?  They're all using memcpy(), so the caller must guarantee that the
> physical addresses are different.

Exactly.  But how does the caller know that?  The memcpy() call is buried
inside the copy_user_page() and/or copy_page().  Users of these functions
should not need to dig that far into an implementation to use this interface
IMO.  Adding some kdoc helps I think.

> 
> > I could not say anything at LSFmm because the Outreachy interns had not been
> > announced but I've selected Fabio to help with the highmem rework through that
> > program.
> 
> Excellent!  I advised Fabio last year, and I think he'll do a sterling
> job this year.

Me too!

> 
> > Would you like Fabio or I to send a patch?  I think the main thing right now is
> > to just drop the memmove_page()
> 
> Sure, just stick my Reported-by: on it.

Absolutely!

Thanks for the report,
Ira