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
