We have counted task's maj/min fault after handle_mm_fault.
break_ksm misses that.
I wanted to check by VM_FAULT_ERROR.
But now break_ksm doesn't handle HWPOISON error.
Signed-off-by: Minchan Kim <[email protected]>
CC: Hugh Dickins <[email protected]>
CC: Izik Eidus <[email protected]>
---
mm/ksm.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/mm/ksm.c b/mm/ksm.c
index 56a0da1..3a1fda4 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -367,9 +367,13 @@ static int break_ksm(struct vm_area_struct *vma, unsigned long addr)
page = follow_page(vma, addr, FOLL_GET);
if (!page)
break;
- if (PageKsm(page))
+ if (PageKsm(page)) {
ret = handle_mm_fault(vma->vm_mm, vma, addr,
FAULT_FLAG_WRITE);
+ if (!(ret & (VM_FAULT_SIGBUS | VM_FAULT_OOM)
+ || current->flags & PF_KTHREAD))
+ current->min_flt++;
+ }
else
ret = VM_FAULT_WRITE;
put_page(page);
--
1.5.6.3
--
Kind regards,
Minchan Kim
On Mon, 11 Jan 2010, Minchan Kim wrote:
> We have counted task's maj/min fault after handle_mm_fault.
> break_ksm misses that.
>
> I wanted to check by VM_FAULT_ERROR.
> But now break_ksm doesn't handle HWPOISON error.
Sorry, no, I just don't see a good reason to add this.
Imagine it this way: these aren't really faults, KSM simply
happens to be using "handle_mm_fault" to achieve what it needs.
(And, of course, if we did add something like this, I'd be
disagreeing with you about which tsk's min_flt to increment.)
Hugh
>
> Signed-off-by: Minchan Kim <[email protected]>
> CC: Hugh Dickins <[email protected]>
> CC: Izik Eidus <[email protected]>
> ---
> mm/ksm.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/mm/ksm.c b/mm/ksm.c
> index 56a0da1..3a1fda4 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -367,9 +367,13 @@ static int break_ksm(struct vm_area_struct *vma, unsigned long addr)
> page = follow_page(vma, addr, FOLL_GET);
> if (!page)
> break;
> - if (PageKsm(page))
> + if (PageKsm(page)) {
> ret = handle_mm_fault(vma->vm_mm, vma, addr,
> FAULT_FLAG_WRITE);
> + if (!(ret & (VM_FAULT_SIGBUS | VM_FAULT_OOM)
> + || current->flags & PF_KTHREAD))
> + current->min_flt++;
> + }
> else
> ret = VM_FAULT_WRITE;
> put_page(page);
> --
> 1.5.6.3
>
>
>
> --
> Kind regards,
> Minchan Kim
Hi, Hugh.
On Tue, Jan 12, 2010 at 8:40 AM, Hugh Dickins
<[email protected]> wrote:
> On Mon, 11 Jan 2010, Minchan Kim wrote:
>
>> We have counted task's maj/min fault after handle_mm_fault.
>> break_ksm misses that.
>>
>> I wanted to check by VM_FAULT_ERROR.
>> But now break_ksm doesn't handle HWPOISON error.
>
> Sorry, no, I just don't see a good reason to add this.
> Imagine it this way: these aren't really faults, KSM simply
> happens to be using "handle_mm_fault" to achieve what it needs.
Why I suggest is handle_mm_fault counts PGFAULT in system.
If we want to get minor fault count in system, we have to calculate
(PGFAULT - PGMAJFAULT).
But we don't count it as either major or minor in this case and GUP case
I doubt it, then I see GUP already handled it tsk->[maj|min]flt.
Although it isn't my point, I thought break_ksm also have to count it
like GUP at least.
Okay. It's not real problem. I found just while I review the code.
I have no objection in your opinion.
But I think it would be better to have a consistency PGFAULT and PGMAJFAULT.
>
> (And, of course, if we did add something like this, I'd be
> disagreeing with you about which tsk's min_flt to increment.)
>
> Hugh
>
>>
>> Signed-off-by: Minchan Kim <[email protected]>
>> CC: Hugh Dickins <[email protected]>
>> CC: Izik Eidus <[email protected]>
>> ---
>> mm/ksm.c | 6 +++++-
>> 1 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/mm/ksm.c b/mm/ksm.c
>> index 56a0da1..3a1fda4 100644
>> --- a/mm/ksm.c
>> +++ b/mm/ksm.c
>> @@ -367,9 +367,13 @@ static int break_ksm(struct vm_area_struct *vma, unsigned long addr)
>> page = follow_page(vma, addr, FOLL_GET);
>> if (!page)
>> break;
>> - if (PageKsm(page))
>> + if (PageKsm(page)) {
>> ret = handle_mm_fault(vma->vm_mm, vma, addr,
>> FAULT_FLAG_WRITE);
>> + if (!(ret & (VM_FAULT_SIGBUS | VM_FAULT_OOM)
>> + || current->flags & PF_KTHREAD))
>> + current->min_flt++;
>> + }
>> else
>> ret = VM_FAULT_WRITE;
>> put_page(page);
>> --
>> 1.5.6.3
>>
>>
>>
>> --
>> Kind regards,
>> Minchan Kim
>
--
Kind regards,
Minchan Kim