Re: [PATCH v1] mm/madvise: pass return code of memory_failure() to userspace

[Naoya Horiguchi]

On Wed, Jan 27, 2016 at 02:59:11PM +0100, Vlastimil Babka wrote:
> [CC += linux-api, linux-man]
> 
> On 01/22/2016 09:27 AM, Naoya Horiguchi wrote:
> > Currently the return value of memory_failure() is not passed to userspace, 
> > which
> > is inconvenient for test programs that want to know the result of error 
> > handling.
> > So let's return it to the caller as we already do in MADV_SOFT_OFFLINE case.
> > 
> > Signed-off-by: Naoya Horiguchi 
> > ---
> >  mm/madvise.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git v4.4-mmotm-2016-01-20-16-10/mm/madvise.c 
> > v4.4-mmotm-2016-01-20-16-10_patched/mm/madvise.c
> > index f56825b..6a77114 100644
> > --- v4.4-mmotm-2016-01-20-16-10/mm/madvise.c
> > +++ v4.4-mmotm-2016-01-20-16-10_patched/mm/madvise.c
> > @@ -555,8 +555,9 @@ static int madvise_hwpoison(int bhv, unsigned long 
> > start, unsigned long end)
> > }
> > pr_info("Injecting memory failure for page %#lx at %#lx\n",
> >page_to_pfn(p), start);
> > -   /* Ignore return value for now */
> > -   memory_failure(page_to_pfn(p), 0, MF_COUNT_INCREASED);
> > +   ret = memory_failure(page_to_pfn(p), 0, MF_COUNT_INCREASED);
> > +   if (ret)
> > +   return ret;
> 
> Can you explain what madvise can newly return for MADV_HWPOISON in which
> situations, for the purposes of updated man page?

OK, this patch newly allows madvise(MADV_HWPOISON) to return EBUSY when
error handling failed due to 2 major reasons:
 - the target page is a page type which memory error handler doesn't support
   (like slab pages, kernel-reserved pages)
 - the memory error handler failed to isolate the target page (for example,
   due to failure in unmapping)

And for man page purpose, errnos of MADV_SOFT_OFFLINE is not documented either.
So let me refer to 2 possible error code from madvise(MADV_SOFT_OFFLINE):
 - EBUSY: failed to isolate from lru list,
 - EIO: failed to migrate the target page to another page.

Thanks,
Naoya Horiguchi

Re: [PATCH v1] mm/madvise: pass return code of memory_failure() to userspace

[Vlastimil Babka]

[CC += linux-api, linux-man]

On 01/22/2016 09:27 AM, Naoya Horiguchi wrote:
> Currently the return value of memory_failure() is not passed to userspace, 
> which
> is inconvenient for test programs that want to know the result of error 
> handling.
> So let's return it to the caller as we already do in MADV_SOFT_OFFLINE case.
> 
> Signed-off-by: Naoya Horiguchi 
> ---
>  mm/madvise.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git v4.4-mmotm-2016-01-20-16-10/mm/madvise.c 
> v4.4-mmotm-2016-01-20-16-10_patched/mm/madvise.c
> index f56825b..6a77114 100644
> --- v4.4-mmotm-2016-01-20-16-10/mm/madvise.c
> +++ v4.4-mmotm-2016-01-20-16-10_patched/mm/madvise.c
> @@ -555,8 +555,9 @@ static int madvise_hwpoison(int bhv, unsigned long start, 
> unsigned long end)
>   }
>   pr_info("Injecting memory failure for page %#lx at %#lx\n",
>  page_to_pfn(p), start);
> - /* Ignore return value for now */
> - memory_failure(page_to_pfn(p), 0, MF_COUNT_INCREASED);
> + ret = memory_failure(page_to_pfn(p), 0, MF_COUNT_INCREASED);
> + if (ret)
> + return ret;

Can you explain what madvise can newly return for MADV_HWPOISON in which
situations, for the purposes of updated man page?

Thanks,
Vlastimil

>   }
>   return 0;
>  }
> 

Re: [PATCH v1] mm/madvise: pass return code of memory_failure() to userspace

[Naoya Horiguchi]

On Wed, Jan 27, 2016 at 02:59:11PM +0100, Vlastimil Babka wrote:
> [CC += linux-api, linux-man]
> 
> On 01/22/2016 09:27 AM, Naoya Horiguchi wrote:
> > Currently the return value of memory_failure() is not passed to userspace, 
> > which
> > is inconvenient for test programs that want to know the result of error 
> > handling.
> > So let's return it to the caller as we already do in MADV_SOFT_OFFLINE case.
> > 
> > Signed-off-by: Naoya Horiguchi 
> > ---
> >  mm/madvise.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git v4.4-mmotm-2016-01-20-16-10/mm/madvise.c 
> > v4.4-mmotm-2016-01-20-16-10_patched/mm/madvise.c
> > index f56825b..6a77114 100644
> > --- v4.4-mmotm-2016-01-20-16-10/mm/madvise.c
> > +++ v4.4-mmotm-2016-01-20-16-10_patched/mm/madvise.c
> > @@ -555,8 +555,9 @@ static int madvise_hwpoison(int bhv, unsigned long 
> > start, unsigned long end)
> > }
> > pr_info("Injecting memory failure for page %#lx at %#lx\n",
> >page_to_pfn(p), start);
> > -   /* Ignore return value for now */
> > -   memory_failure(page_to_pfn(p), 0, MF_COUNT_INCREASED);
> > +   ret = memory_failure(page_to_pfn(p), 0, MF_COUNT_INCREASED);
> > +   if (ret)
> > +   return ret;
> 
> Can you explain what madvise can newly return for MADV_HWPOISON in which
> situations, for the purposes of updated man page?

OK, this patch newly allows madvise(MADV_HWPOISON) to return EBUSY when
error handling failed due to 2 major reasons:
 - the target page is a page type which memory error handler doesn't support
   (like slab pages, kernel-reserved pages)
 - the memory error handler failed to isolate the target page (for example,
   due to failure in unmapping)

And for man page purpose, errnos of MADV_SOFT_OFFLINE is not documented either.
So let me refer to 2 possible error code from madvise(MADV_SOFT_OFFLINE):
 - EBUSY: failed to isolate from lru list,
 - EIO: failed to migrate the target page to another page.

Thanks,
Naoya Horiguchi

Re: [PATCH v1] mm/madvise: pass return code of memory_failure() to userspace

[Vlastimil Babka]

[CC += linux-api, linux-man]

On 01/22/2016 09:27 AM, Naoya Horiguchi wrote:
> Currently the return value of memory_failure() is not passed to userspace, 
> which
> is inconvenient for test programs that want to know the result of error 
> handling.
> So let's return it to the caller as we already do in MADV_SOFT_OFFLINE case.
> 
> Signed-off-by: Naoya Horiguchi 
> ---
>  mm/madvise.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git v4.4-mmotm-2016-01-20-16-10/mm/madvise.c 
> v4.4-mmotm-2016-01-20-16-10_patched/mm/madvise.c
> index f56825b..6a77114 100644
> --- v4.4-mmotm-2016-01-20-16-10/mm/madvise.c
> +++ v4.4-mmotm-2016-01-20-16-10_patched/mm/madvise.c
> @@ -555,8 +555,9 @@ static int madvise_hwpoison(int bhv, unsigned long start, 
> unsigned long end)
>   }
>   pr_info("Injecting memory failure for page %#lx at %#lx\n",
>  page_to_pfn(p), start);
> - /* Ignore return value for now */
> - memory_failure(page_to_pfn(p), 0, MF_COUNT_INCREASED);
> + ret = memory_failure(page_to_pfn(p), 0, MF_COUNT_INCREASED);
> + if (ret)
> + return ret;

Can you explain what madvise can newly return for MADV_HWPOISON in which
situations, for the purposes of updated man page?

Thanks,
Vlastimil

>   }
>   return 0;
>  }
> 

Re: [PATCH v1] mm/madvise: pass return code of memory_failure() to userspace

[Naoya Horiguchi]

On Tue, Jan 26, 2016 at 03:27:58PM -0800, Andrew Morton wrote:
> On Fri, 22 Jan 2016 17:27:57 +0900 Naoya Horiguchi 
>  wrote:
> 
> > Currently the return value of memory_failure() is not passed to userspace, 
> > which
> > is inconvenient for test programs that want to know the result of error 
> > handling.
> > So let's return it to the caller as we already do in MADV_SOFT_OFFLINE case.
> 
> I updated this to mention that it's for madvise(MADV_HWPOISON):
> 
> : Currently the return value of memory_failure() is not passed to userspace
> : when madvise(MADV_HWPOISON) is used.  This is inconvenient for test
> : programs that want to know the result of error handling.  So let's return
> : it to the caller as we already do in the MADV_SOFT_OFFLINE case.

Thank you.

> btw, MADV_SOFT_OFFLINE and MADV_HWPOISON are not documented in that
> comment block over sys_madvise().  Fixy please?  You might want to
> check that no other MADV_foo values have been omitted.

OK, I posted the fix patch just now, which also updates about some other
madvices.

Thanks,
Naoya Horiguchi

Re: [PATCH v1] mm/madvise: pass return code of memory_failure() to userspace

[Andrew Morton]

On Fri, 22 Jan 2016 17:27:57 +0900 Naoya Horiguchi  
wrote:

> Currently the return value of memory_failure() is not passed to userspace, 
> which
> is inconvenient for test programs that want to know the result of error 
> handling.
> So let's return it to the caller as we already do in MADV_SOFT_OFFLINE case.

I updated this to mention that it's for madvise(MADV_HWPOISON):

: Currently the return value of memory_failure() is not passed to userspace
: when madvise(MADV_HWPOISON) is used.  This is inconvenient for test
: programs that want to know the result of error handling.  So let's return
: it to the caller as we already do in the MADV_SOFT_OFFLINE case.

btw, MADV_SOFT_OFFLINE and MADV_HWPOISON are not documented in that
comment block over sys_madvise().  Fixy please?  You might want to
check that no other MADV_foo values have been omitted.

Re: [PATCH v1] mm/madvise: pass return code of memory_failure() to userspace

[Andrew Morton]

On Fri, 22 Jan 2016 17:27:57 +0900 Naoya Horiguchi  
wrote:

> Currently the return value of memory_failure() is not passed to userspace, 
> which
> is inconvenient for test programs that want to know the result of error 
> handling.
> So let's return it to the caller as we already do in MADV_SOFT_OFFLINE case.

I updated this to mention that it's for madvise(MADV_HWPOISON):

: Currently the return value of memory_failure() is not passed to userspace
: when madvise(MADV_HWPOISON) is used.  This is inconvenient for test
: programs that want to know the result of error handling.  So let's return
: it to the caller as we already do in the MADV_SOFT_OFFLINE case.

btw, MADV_SOFT_OFFLINE and MADV_HWPOISON are not documented in that
comment block over sys_madvise().  Fixy please?  You might want to
check that no other MADV_foo values have been omitted.

Re: [PATCH v1] mm/madvise: pass return code of memory_failure() to userspace

[Naoya Horiguchi]

On Tue, Jan 26, 2016 at 03:27:58PM -0800, Andrew Morton wrote:
> On Fri, 22 Jan 2016 17:27:57 +0900 Naoya Horiguchi 
>  wrote:
> 
> > Currently the return value of memory_failure() is not passed to userspace, 
> > which
> > is inconvenient for test programs that want to know the result of error 
> > handling.
> > So let's return it to the caller as we already do in MADV_SOFT_OFFLINE case.
> 
> I updated this to mention that it's for madvise(MADV_HWPOISON):
> 
> : Currently the return value of memory_failure() is not passed to userspace
> : when madvise(MADV_HWPOISON) is used.  This is inconvenient for test
> : programs that want to know the result of error handling.  So let's return
> : it to the caller as we already do in the MADV_SOFT_OFFLINE case.

Thank you.

> btw, MADV_SOFT_OFFLINE and MADV_HWPOISON are not documented in that
> comment block over sys_madvise().  Fixy please?  You might want to
> check that no other MADV_foo values have been omitted.

OK, I posted the fix patch just now, which also updates about some other
madvices.

Thanks,
Naoya Horiguchi

[PATCH v1] mm/madvise: pass return code of memory_failure() to userspace

[Naoya Horiguchi]

Currently the return value of memory_failure() is not passed to userspace, which
is inconvenient for test programs that want to know the result of error 
handling.
So let's return it to the caller as we already do in MADV_SOFT_OFFLINE case.

Signed-off-by: Naoya Horiguchi 
---
 mm/madvise.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git v4.4-mmotm-2016-01-20-16-10/mm/madvise.c 
v4.4-mmotm-2016-01-20-16-10_patched/mm/madvise.c
index f56825b..6a77114 100644
--- v4.4-mmotm-2016-01-20-16-10/mm/madvise.c
+++ v4.4-mmotm-2016-01-20-16-10_patched/mm/madvise.c
@@ -555,8 +555,9 @@ static int madvise_hwpoison(int bhv, unsigned long start, 
unsigned long end)
}
pr_info("Injecting memory failure for page %#lx at %#lx\n",
   page_to_pfn(p), start);
-   /* Ignore return value for now */
-   memory_failure(page_to_pfn(p), 0, MF_COUNT_INCREASED);
+   ret = memory_failure(page_to_pfn(p), 0, MF_COUNT_INCREASED);
+   if (ret)
+   return ret;
}
return 0;
 }
-- 
2.7.0

[PATCH v1] mm/madvise: pass return code of memory_failure() to userspace

[Naoya Horiguchi]

Currently the return value of memory_failure() is not passed to userspace, which
is inconvenient for test programs that want to know the result of error 
handling.
So let's return it to the caller as we already do in MADV_SOFT_OFFLINE case.

Signed-off-by: Naoya Horiguchi 
---
 mm/madvise.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git v4.4-mmotm-2016-01-20-16-10/mm/madvise.c 
v4.4-mmotm-2016-01-20-16-10_patched/mm/madvise.c
index f56825b..6a77114 100644
--- v4.4-mmotm-2016-01-20-16-10/mm/madvise.c
+++ v4.4-mmotm-2016-01-20-16-10_patched/mm/madvise.c
@@ -555,8 +555,9 @@ static int madvise_hwpoison(int bhv, unsigned long start, 
unsigned long end)
}
pr_info("Injecting memory failure for page %#lx at %#lx\n",
   page_to_pfn(p), start);
-   /* Ignore return value for now */
-   memory_failure(page_to_pfn(p), 0, MF_COUNT_INCREASED);
+   ret = memory_failure(page_to_pfn(p), 0, MF_COUNT_INCREASED);
+   if (ret)
+   return ret;
}
return 0;
 }
-- 
2.7.0