On Wed, Feb 08, 2023 at 03:00:59PM -0800, Jiaqi Yan wrote: > On Wed, Feb 8, 2023 at 3:45 AM Alexander Potapenko <glider@xxxxxxxxxx> wrote: > > > > On Tue, Feb 7, 2023 at 7:20 PM Jiaqi Yan <jiaqiyan@xxxxxxxxxx> wrote: > > > > > > Pinging KMSAN experts, for the general guidance of > > > kmsan_copy_page_meta vs kmsan_unpoison_memory > > > > Oh, sorry, I've missed the previous email. > > NP, thanks for jumping in :) > > > > > copy_mc_user_highpage() is expected to copy data from the user page, > > for which no metadata is ever allocated. > > Therefore we just initialize the destination shadow with zeros instead > > of copying anything. > > > > kmsan_copy_page_meta() is used when the metadata is copied between two > > kernel pages, therefore it handles the cases when page->kmsan_shadow > > is NULL for the source and destination pages. > > > > It might be a good idea to use kmsan_copy_page_meta() in both cases, > > but to do that I want to better understand what happens when > > kmap_local_page(from) is called in copy_mc_user_highpage(). > > Where does the corresponding struct page come from? > > Kirill can correct me, but I think khugepaged always copies user pages > because it is trying to convert raw pages to THP for better userspace > application performance. Therefore khugepaged should only need > copy_mc_user_highpage(), for both file-backed and anonymous memory > pages. > > However, copy_mc_user_highpage() needs both vaddr and vma, so it is a > little bit hard to use it in collapse_file (i.e. in the file-backed > case): > 1. vma is not carried over to collapse_file from khugepaged_scan_mm_slot > 2. collapse_file is not directly iterating with vaddrs of pages to be copied > > (Although both vaddr and vma are unused auguments in > copy_mc_user_highpage, I think for cleanness, the caller e.g. > khugepaged should feed valid values). It is not unused for !copy_mc_to_kernel case (basically everything but x86 and power). And it is used to flush caches. The fact that you don't have it in your implementation *may* indicate a problem. > So my patchset uses copy_mc_page(and kmsan_copy_page_meta) for both > file-backed and anon memory pages. I guess as long as > kmsan_copy_page_meta doesn't do anything unexpected for user pages (at > least from my reading), we are good? > -- Kiryl Shutsemau / Kirill A. Shutemov
