lkml.org 
[lkml]   [2018]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH -V5 RESEND 03/21] swap: Support PMD swap mapping in swap_duplicate()
Date
Daniel Jordan <daniel.m.jordan@oracle.com> writes:

> On Fri, Sep 28, 2018 at 04:19:03PM +0800, Huang, Ying wrote:
>> Daniel Jordan <daniel.m.jordan@oracle.com> writes:
>> > One way is to change
>> > copy_one_pte's return to int so we can just pass the error code back to
>> > copy_pte_range so it knows whether to try adding the continuation.
>>
>> There may be even more problems. After add_swap_count_continuation(),
>> copy_one_pte() will be retried, and the CPU may hang with dead loop.
>
> That's true, it would do that.
>
>> But before the changes in this patchset, the behavior is,
>> __swap_duplicate() return an error that isn't -ENOMEM, such as -EEXIST.
>> Then copy_one_pte() would thought the operation has been done
>> successfully, and go to call set_pte_at(). This will cause the system
>> state become inconsistent, and the system may panic or hang somewhere
>> later.
>>
>> So per my understanding, if we thought page table corruption isn't a
>> real problem (that is, __swap_duplicate() will never return e.g. -EEXIST
>> if copied by copy_one_pte() indirectly), both the original and the new
>> code should be OK.
>>
>> If we thought it is a real problem, we need to fix the original code and
>> keep it fixed in the new code. Do you agree?
>
> Yes, if it was a real problem, which seems less and less the case the more I
> stare at this.
>
>> There's several ways to fix the problem. But the page table shouldn't
>> be corrupted in practice, unless there's some programming error. So I
>> suggest to make it as simple as possible via adding,
>>
>> VM_BUG_ON(error != -ENOMEM);
>>
>> in swap_duplicate().
>>
>> Do you agree?
>
> Yes, I'm ok with that, adding in -ENOTDIR along with it.

Sure. I will do this.

> The error handling in __swap_duplicate (before this series) still leaves
> something to be desired IMHO. Why all the different returns when callers
> ignore them or only specifically check for -ENOMEM or -EEXIST? Could maybe
> stand a cleanup, but outside this series.

Yes. Maybe. I guess you will work on this?

Best Regards,
Huang, Ying

\
 
 \ /
  Last update: 2018-09-29 02:51    [W:0.118 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site