On Fri, Nov 18, 2022 at 8:46 AM Luck, Tony <tony.luck@xxxxxxxxx> wrote:
>
> +/*
> + * Machine check exception handled version of copy_highpage.
> + * Return true if copying page content failed; otherwise false.
> + * Note handling #MC requires arch opt-in.
> + */
> +static inline bool copy_highpage_mc(struct page *to, struct page *from)
> +{
> + char *vfrom, *vto;
> + unsigned long ret;
> +
> + vfrom = kmap_local_page(from);
> + vto = kmap_local_page(to);
> + ret = copy_mc_to_kernel(vto, vfrom, PAGE_SIZE);
> + kunmap_local(vto);
> + kunmap_local(vfrom);
> +
> + return ret > 0;
> +}
>
> Andrew Morton took my patches to recover from copy-on-write faults
> into the -mm tree. They are now sitting in linux-next (hopefully) ready
> for the next merge window.
>
> I had copied this function from an earlier version of your patch, but
> there was a suggestion that copy_mc_user_highpage() would be a
> more consistent name with other copy code that can handle machine
> checks. There was also an upstream change to copy_highpage()
> that needed to be reflected here.
Thanks for pointing this out, Tony. Is it the kmsan_copy_page_meta(to,
from) call in copy_highpage()? I will make sure to include it in
copy_highpage_mc(). If there is more, can you provide lore pointers?
>
> Net result, this version is already in flight:
>
> static inline int copy_mc_user_highpage(struct page *to, struct page *from,
> unsigned long vaddr, struct vm_area_struct *vma)
> {
> unsigned long ret;
> char *vfrom, *vto;
>
> vfrom = kmap_local_page(from);
> vto = kmap_local_page(to);
> ret = copy_mc_to_kernel(vto, vfrom, PAGE_SIZE);
> if (!ret)
> kmsan_unpoison_memory(page_address(to), PAGE_SIZE);
Does it make sense to kmsan_unpoison_memory in copy_highpage? (and in
copy_mc_highpage if no MC happens?)
> kunmap_local(vto);
> kunmap_local(vfrom);
>
> return ret;
> }
>
> -Tony
While I am ok to switch to copy_mc_user_highpage for this commit
(given __collapse_huge_page_copy originally uses copy_user_highpage),
the question is, Yang Shi suggested in a previous version to unify the
copy routine used by both __collapse_huge_page_copy (using
copy_user_highpage) and collapse_file (using copy_highpage). I prefer
MC-aware copy_highpage because collapse_file may have a hard time to
adapt to the vaddr and vma in copy_user_highpage's interface, and
neither collapse_file nor __collapse_huge_page_copy gain anything from
passing vaddr and vma (correct me if I am wrong).
So I would still prefer copy_mc_highpage (after renaming and adding
kmsan_xxx calls) to be used by both __collapse_huge_page_copy and
collapse_file. What are your opinions, Tony and Yang?
