Xen devel - Jan 2012 - [PATCH] xenpaging: unify return value in nominate and evict

# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1327607757 -3600
# Node ID ebc68ae15103061fa554745efb652b7d7e7b8e89
# Parent  266a12304601226213a57e39cc11aa075acdfb58
xenpaging: unify return value in nominate and evict

Let p2m_mem_paging_nominate and p2m_mem_paging_evict return just one
error number. EINVAL is not very helpful in case of nominate, it can
happen if the pager tries to nominate a ballooned page. In this case the
gfn is not backed by a mfn, the pager can not know that.  Similar with
evict, anything can happen between nominate and evict.

This change helps the pager to decide if the returned error is from the
function itself, or if it happend earlier. In the latter case, it is
most likely fatal and should be handled as such.
nominate returns EAGAIN, evict EBUSY.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

diff -r 266a12304601 -r ebc68ae15103 xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -735,19 +735,17 @@ int p2m_mem_paging_nominate(struct domai
     p2m_type_t p2mt;
     p2m_access_t a;
     mfn_t mfn;
-    int ret;
+    int ret = -EAGAIN;
 
     p2m_lock(p2m);
 
     mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, p2m_query, NULL);
 
     /* Check if mfn is valid */
-    ret = -EINVAL;
     if ( !mfn_valid(mfn) )
         goto out;
 
     /* Check p2m type */
-    ret = -EAGAIN;
     if ( !p2m_is_pageable(p2mt) )
         goto out;
 
@@ -799,7 +797,7 @@ int p2m_mem_paging_evict(struct domain *
     p2m_access_t a;
     mfn_t mfn;
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
-    int ret = -EINVAL;
+    int ret = -EBUSY;
 
     p2m_lock(p2m);
 
@@ -812,7 +810,6 @@ int p2m_mem_paging_evict(struct domain *
     if ( p2mt != p2m_ram_paging_out )
         goto out;
 
-    ret = -EBUSY;
     /* Get the page so it doesn''t get modified under Xen''s
feet */
     page = mfn_to_page(mfn);
     if ( unlikely(!get_page(page, d)) )

At 20:56 +0100 on 26 Jan (1327611377), Olaf Hering
wrote:
> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1327607757 -3600
> # Node ID ebc68ae15103061fa554745efb652b7d7e7b8e89
> # Parent  266a12304601226213a57e39cc11aa075acdfb58
> xenpaging: unify return value in nominate and evict
> 
> Let p2m_mem_paging_nominate and p2m_mem_paging_evict return just one
> error number. EINVAL is not very helpful in case of nominate, it can
> happen if the pager tries to nominate a ballooned page. In this case the
> gfn is not backed by a mfn, the pager can not know that.  Similar with
> evict, anything can happen between nominate and evict.
> 
> This change helps the pager to decide if the returned error is from the
> function itself, or if it happend earlier. In the latter case, it is
> most likely fatal and should be handled as such.
> nominate returns EAGAIN, evict EBUSY.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> diff -r 266a12304601 -r ebc68ae15103 xen/arch/x86/mm/p2m.c
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -735,19 +735,17 @@ int p2m_mem_paging_nominate(struct domai
>      p2m_type_t p2mt;
>      p2m_access_t a;
>      mfn_t mfn;
> -    int ret;
> +    int ret = -EAGAIN;
>  
>      p2m_lock(p2m);
>  
>      mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, p2m_query, NULL);
>  
>      /* Check if mfn is valid */
> -    ret = -EINVAL;
>      if ( !mfn_valid(mfn) )
>          goto out;
>  
I don''t think EAGAIN is the right thing to return here. 
There''s no
reason to think that retrying an invalid GFN will work next time. 

I''d be happy for these cases all to return the same code, but maybe
EINVAL would be a better choice. 

Cheers,

Tim.

On Mon, Jan 30, Tim Deegan wrote:
> I don''t think EAGAIN is the right thing to return here. 
There''s no
> reason to think that retrying an invalid GFN will work next time. 
> 
> I''d be happy for these cases all to return the same code, but
maybe
> EINVAL would be a better choice. 
EINVAL is returned by upper layers already. I think both nominate and
evict should return EBUSY in case of failure, this is most likely unique
enough to mean "request reached target function, and failed.".

Olaf

At 11:24 +0100 on 30 Jan (1327922652), Olaf Hering
wrote:
> On Mon, Jan 30, Tim Deegan wrote:
> 
> > I don''t think EAGAIN is the right thing to return here. 
There''s no
> > reason to think that retrying an invalid GFN will work next time. 
> > 
> > I''d be happy for these cases all to return the same code, but
maybe
> > EINVAL would be a better choice. 
> 
> EINVAL is returned by upper layers already. I think both nominate and
> evict should return EBUSY in case of failure, this is most likely unique
> enough to mean "request reached target function, and failed.".
Fair enough; EBUSY will do.

Tim.

# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1327925194 -3600
# Node ID 17e25c8b6045cd7246e4ee5e914efba6f44a26fd
# Parent  2577131785ab86c95e6631a01bff04c664eac876
xenpaging: unify return value in nominate and evict

Let p2m_mem_paging_nominate and p2m_mem_paging_evict return just one
error number. EINVAL is not very helpful in case of nominate, it can
happen if the pager tries to nominate a ballooned page. In this case the
gfn is not backed by a mfn, the pager can not know that.  Similar with
evict, anything can happen between nominate and evict.

This change helps the pager to decide if the returned error is from the
function itself, or if it happend earlier. In the latter case, it is
most likely fatal and should be handled as such.
nominate and evict return EBUSY, which is supposed to mean
"pager request reached target function, and failed."

Signed-off-by: Olaf Hering <olaf@aepfle.de>

diff -r 2577131785ab -r 17e25c8b6045 xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -735,19 +735,17 @@ int p2m_mem_paging_nominate(struct domai
     p2m_type_t p2mt;
     p2m_access_t a;
     mfn_t mfn;
-    int ret;
+    int ret = -EBUSY;
 
     p2m_lock(p2m);
 
     mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, p2m_query, NULL);
 
     /* Check if mfn is valid */
-    ret = -EINVAL;
     if ( !mfn_valid(mfn) )
         goto out;
 
     /* Check p2m type */
-    ret = -EAGAIN;
     if ( !p2m_is_pageable(p2mt) )
         goto out;
 
@@ -799,7 +797,7 @@ int p2m_mem_paging_evict(struct domain *
     p2m_access_t a;
     mfn_t mfn;
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
-    int ret = -EINVAL;
+    int ret = -EBUSY;
 
     p2m_lock(p2m);
 
@@ -812,7 +810,6 @@ int p2m_mem_paging_evict(struct domain *
     if ( p2mt != p2m_ram_paging_out )
         goto out;
 
-    ret = -EBUSY;
     /* Get the page so it doesn''t get modified under Xen''s
feet */
     page = mfn_to_page(mfn);
     if ( unlikely(!get_page(page, d)) )

At 13:07 +0100 on 30 Jan (1327928841), Olaf Hering
wrote:
> xenpaging: unify return value in nominate and evict
> 
> Let p2m_mem_paging_nominate and p2m_mem_paging_evict return just one
> error number. EINVAL is not very helpful in case of nominate, it can
> happen if the pager tries to nominate a ballooned page. In this case the
> gfn is not backed by a mfn, the pager can not know that.  Similar with
> evict, anything can happen between nominate and evict.
> 
> This change helps the pager to decide if the returned error is from the
> function itself, or if it happend earlier. In the latter case, it is
> most likely fatal and should be handled as such.
> nominate and evict return EBUSY, which is supposed to mean
> "pager request reached target function, and failed."
Applied, thanks.

Tim.