Re: [Xen-devel] [PATCHv2 10/10] xen/balloon: pre-allocate p2m entries for ballooned pages

2015-07-27 Thread David Vrabel
On 27/07/15 12:01, Julien Grall wrote:
> On 27/07/15 10:30, David Vrabel wrote:
>> On 25/07/15 00:21, Julien Grall wrote:
>>> On 24/07/2015 12:47, David Vrabel wrote:
 @@ -550,6 +551,11 @@ int alloc_xenballooned_pages(int nr_pages, struct
 page **pages)
   page = balloon_retrieve(true);
   if (page) {
   pages[pgno++] = page;
 +#ifdef CONFIG_XEN_HAVE_PVMMU
 +ret = xen_alloc_p2m_entry(page_to_pfn(page));
>>>
>>> Don't you want to call this function only when the guest is not using
>>> auto-translated physmap?
>>
>> xen_alloc_p2m_entry() is a nop in auto-xlate guests, so no need for an
>> additional check here.
> 
> I don't have the impression it's the case or it's not obvious.

Oops. You're right.  I'll add a

if (xen_feature(XENFEAT_auto_translated_physmap))
return true;

Check at the top.

David


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCHv2 10/10] xen/balloon: pre-allocate p2m entries for ballooned pages

2015-07-27 Thread Julien Grall
On 27/07/15 10:30, David Vrabel wrote:
> On 25/07/15 00:21, Julien Grall wrote:
>> On 24/07/2015 12:47, David Vrabel wrote:
>>> @@ -550,6 +551,11 @@ int alloc_xenballooned_pages(int nr_pages, struct
>>> page **pages)
>>>   page = balloon_retrieve(true);
>>>   if (page) {
>>>   pages[pgno++] = page;
>>> +#ifdef CONFIG_XEN_HAVE_PVMMU
>>> +ret = xen_alloc_p2m_entry(page_to_pfn(page));
>>
>> Don't you want to call this function only when the guest is not using
>> auto-translated physmap?
> 
> xen_alloc_p2m_entry() is a nop in auto-xlate guests, so no need for an
> additional check here.

I don't have the impression it's the case or it's not obvious.

For instance xen_p2m_addr, used within with the xen_alloc_p2m_entry (old
name alloc_p2m) is never set for auto-xlate guests. Therefore the value
is 0.

Same for p2m_identity and p2m_missing & co.

Regards,

-- 
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCHv2 10/10] xen/balloon: pre-allocate p2m entries for ballooned pages

2015-07-27 Thread David Vrabel
On 25/07/15 00:21, Julien Grall wrote:
> On 24/07/2015 12:47, David Vrabel wrote:
>> @@ -550,6 +551,11 @@ int alloc_xenballooned_pages(int nr_pages, struct
>> page **pages)
>>   page = balloon_retrieve(true);
>>   if (page) {
>>   pages[pgno++] = page;
>> +#ifdef CONFIG_XEN_HAVE_PVMMU
>> +ret = xen_alloc_p2m_entry(page_to_pfn(page));
> 
> Don't you want to call this function only when the guest is not using
> auto-translated physmap?

xen_alloc_p2m_entry() is a nop in auto-xlate guests, so no need for an
additional check here.

David

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCHv2 10/10] xen/balloon: pre-allocate p2m entries for ballooned pages

2015-07-24 Thread Julien Grall

Hi David,

On 24/07/2015 12:47, David Vrabel wrote:

Pages returned by alloc_xenballooned_pages() will be used for grant
mapping which will call set_phys_to_machine() (in PV guests).

Ballooned pages are set as INVALID_P2M_ENTRY in the p2m and thus may
be using the (shared) missing tables and a subsequent
set_phys_to_machine() will need to allocate new tables.

Since the grant mapping may be done from a context that cannot sleep,
the p2m entries must already be allocated.

Signed-off-by: David Vrabel 
---
  drivers/xen/balloon.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index fd6970f3..8932d10 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -541,6 +541,7 @@ int alloc_xenballooned_pages(int nr_pages, struct page 
**pages)
  {
int pgno = 0;
struct page *page;
+   int ret = -ENOMEM;

mutex_lock(&balloon_mutex);

@@ -550,6 +551,11 @@ int alloc_xenballooned_pages(int nr_pages, struct page 
**pages)
page = balloon_retrieve(true);
if (page) {
pages[pgno++] = page;
+#ifdef CONFIG_XEN_HAVE_PVMMU
+   ret = xen_alloc_p2m_entry(page_to_pfn(page));


Don't you want to call this function only when the guest is not using 
auto-translated physmap?



+   if (ret < 0)
+   goto out_undo;
+#endif
} else {
enum bp_state st;

@@ -576,7 +582,7 @@ int alloc_xenballooned_pages(int nr_pages, struct page 
**pages)
   out_undo:
mutex_unlock(&balloon_mutex);
free_xenballooned_pages(pgno, pages);
-   return -ENOMEM;
+   return ret;
  }
  EXPORT_SYMBOL(alloc_xenballooned_pages);




Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


[Xen-devel] [PATCHv2 10/10] xen/balloon: pre-allocate p2m entries for ballooned pages

2015-07-24 Thread David Vrabel
Pages returned by alloc_xenballooned_pages() will be used for grant
mapping which will call set_phys_to_machine() (in PV guests).

Ballooned pages are set as INVALID_P2M_ENTRY in the p2m and thus may
be using the (shared) missing tables and a subsequent
set_phys_to_machine() will need to allocate new tables.

Since the grant mapping may be done from a context that cannot sleep,
the p2m entries must already be allocated.

Signed-off-by: David Vrabel 
---
 drivers/xen/balloon.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index fd6970f3..8932d10 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -541,6 +541,7 @@ int alloc_xenballooned_pages(int nr_pages, struct page 
**pages)
 {
int pgno = 0;
struct page *page;
+   int ret = -ENOMEM;
 
mutex_lock(&balloon_mutex);
 
@@ -550,6 +551,11 @@ int alloc_xenballooned_pages(int nr_pages, struct page 
**pages)
page = balloon_retrieve(true);
if (page) {
pages[pgno++] = page;
+#ifdef CONFIG_XEN_HAVE_PVMMU
+   ret = xen_alloc_p2m_entry(page_to_pfn(page));
+   if (ret < 0)
+   goto out_undo;
+#endif
} else {
enum bp_state st;
 
@@ -576,7 +582,7 @@ int alloc_xenballooned_pages(int nr_pages, struct page 
**pages)
  out_undo:
mutex_unlock(&balloon_mutex);
free_xenballooned_pages(pgno, pages);
-   return -ENOMEM;
+   return ret;
 }
 EXPORT_SYMBOL(alloc_xenballooned_pages);
 
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel