Re: [PATCH -next resend v3] mm: hwposion: support recovery from ksm_might_need_to_copy()

[Miaohe Lin <linmiaohe@huawei.com>]

On 2022/12/16 9:47, HORIGUCHI NAOYA(堀口 直也) wrote:
> On Tue, Dec 13, 2022 at 08:05:23PM +0800, Kefeng Wang wrote:
>> When the kernel copy a page from ksm_might_need_to_copy(), but runs
>> into an uncorrectable error, it will crash since poisoned page is
>> consumed by kernel, this is similar to Copy-on-write poison recovery,
> 
> Maybe you mean "this is similar to the issue recently fixed by
> Copy-on-write poison recovery."?  And if this sentence ends here,
> please put "." instead of ",".
> 
>> When an error is detected during the page copy, return VM_FAULT_HWPOISON
>> in do_swap_page(), and install a hwpoison entry in unuse_pte() when
>> swapoff, which help us to avoid system crash.  Note, memory failure on
>> a KSM page will be skipped, but still call memory_failure_queue() to
>> be consistent with general memory failure process.
> 
> Thank you for the work.  I have a few comment below ...

Thanks both.

>> -	if (unlikely(!PageUptodate(page))) {
>> -		pte_t pteval;
>> +	if (hwposioned || !PageUptodate(page)) {
>> +		swp_entry_t swp_entry;
>>  
>>  		dec_mm_counter(vma->vm_mm, MM_SWAPENTS);
>> -		pteval = swp_entry_to_pte(make_swapin_error_entry());
>> -		set_pte_at(vma->vm_mm, addr, pte, pteval);
>> -		swap_free(entry);
>> +		if (hwposioned) {
>> +			swp_entry = make_hwpoison_entry(swapcache);
>> +			page = swapcache;
> 
> This might work for the process accessing to the broken page, but ksm
> pages are likely to be shared by multiple processes, so it would be
> much nicer if you can convert all mapping entries for the error ksm page
> into hwpoisoned ones.  Maybe in this thorough approach,
> hwpoison_user_mappings() is updated to call try_to_unmap() for ksm pages.
> But it's not necessary to do this together with applying mcsafe-memcpy,
> because recovery action and mcsafe-memcpy can be done independently.
> 

I'm afraid leaving the ksm page in the cache will repeatedly trigger uncorrectable error for the
same page if ksm pages are shared by multiple processes. This might reach the hardware threshold
and result in fatal uncorrectable error (thus casuing system to panic). So IMHO it might be better
to check if page is hwpoisoned before calling ksm_might_need_to_copy() if above thorough approach
is not implemented. But I can easily be wrong.

Thanks,
Miaohe Lin