[PATCH] xen: release lock occasionally during ballooning

[PATCH] xen: release lock occasionally during ballooning

[Juergen Gross]

When dom0 is being ballooned balloon_process() will hold the balloon
mutex until it is finished. This will block e.g. creation of new
domains as the device backends for the new domain need some
autoballooned pages for the ring buffers.

Avoid this by releasing the balloon mutex from time to time during
ballooning. Add a state variable to indicate one balloon_process()
is active to avoid multiple balloon processes fighting for the mutex.

Instead of open coding it, just use cond_resched().

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/xen/balloon.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index fd93369..e6d9eee 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -481,9 +481,16 @@ static void balloon_process(struct work_struct *work)
 {
 	enum bp_state state = BP_DONE;
 	long credit;
+	static bool active;
 
 	mutex_lock(&balloon_mutex);
 
+	if (active) {
+		mutex_unlock(&balloon_mutex);
+		return;
+	}
+	active = true;
+
 	do {
 		credit = current_credit();
 
@@ -499,12 +506,16 @@ static void balloon_process(struct work_struct *work)
 
 		state = update_schedule(state);
 
-#ifndef CONFIG_PREEMPT
-		if (need_resched())
-			schedule();
-#endif
+		mutex_unlock(&balloon_mutex);
+
+		cond_resched();
+
+		mutex_lock(&balloon_mutex);
+
 	} while (credit && state == BP_DONE);
 
+	active = false;
+
 	/* Schedule more work if there is some still to be done. */
 	if (state == BP_EAGAIN)
 		schedule_delayed_work(&balloon_worker, balloon_stats.schedule_delay * HZ);
-- 
2.1.4

Re: [PATCH] xen: release lock occasionally during ballooning

[Boris Ostrovsky]

On 07/10/2015 10:42 AM, Juergen Gross wrote:
> When dom0 is being ballooned balloon_process() will hold the balloon
> mutex until it is finished. This will block e.g. creation of new
> domains as the device backends for the new domain need some
> autoballooned pages for the ring buffers.
>
> Avoid this by releasing the balloon mutex from time to time during
> ballooning. Add a state variable to indicate one balloon_process()
> is active to avoid multiple balloon processes fighting for the mutex.
>
> Instead of open coding it, just use cond_resched().
>
> Signed-off-by: Juergen Gross <jgross@suse.com>


Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>


> ---
>   drivers/xen/balloon.c | 19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index fd93369..e6d9eee 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -481,9 +481,16 @@ static void balloon_process(struct work_struct *work)
>   {
>   	enum bp_state state = BP_DONE;
>   	long credit;
> +	static bool active;
>   
>   	mutex_lock(&balloon_mutex);
>   
> +	if (active) {
> +		mutex_unlock(&balloon_mutex);
> +		return;
> +	}
> +	active = true;
> +
>   	do {
>   		credit = current_credit();
>   
> @@ -499,12 +506,16 @@ static void balloon_process(struct work_struct *work)
>   
>   		state = update_schedule(state);
>   
> -#ifndef CONFIG_PREEMPT
> -		if (need_resched())
> -			schedule();
> -#endif
> +		mutex_unlock(&balloon_mutex);
> +
> +		cond_resched();
> +
> +		mutex_lock(&balloon_mutex);
> +
>   	} while (credit && state == BP_DONE);
>   
> +	active = false;
> +
>   	/* Schedule more work if there is some still to be done. */
>   	if (state == BP_EAGAIN)
>   		schedule_delayed_work(&balloon_worker, balloon_stats.schedule_delay * HZ);

Re: [Xen-devel] [PATCH] xen: release lock occasionally during ballooning

[David Vrabel]

On 10/07/15 15:42, Juergen Gross wrote:
> When dom0 is being ballooned balloon_process() will hold the balloon
> mutex until it is finished. This will block e.g. creation of new
> domains as the device backends for the new domain need some
> autoballooned pages for the ring buffers.
> 
> Avoid this by releasing the balloon mutex from time to time during
> ballooning. Add a state variable to indicate one balloon_process()
> is active to avoid multiple balloon processes fighting for the mutex.

Is this state variable necessary? balloon_process() is a work item so
there should only be one instance of it running anyway, yes?

David

Re: [Xen-devel] [PATCH] xen: release lock occasionally during ballooning

[Juergen Gross]

On 07/20/2015 12:15 PM, David Vrabel wrote:
> On 10/07/15 15:42, Juergen Gross wrote:
>> When dom0 is being ballooned balloon_process() will hold the balloon
>> mutex until it is finished. This will block e.g. creation of new
>> domains as the device backends for the new domain need some
>> autoballooned pages for the ring buffers.
>>
>> Avoid this by releasing the balloon mutex from time to time during
>> ballooning. Add a state variable to indicate one balloon_process()
>> is active to avoid multiple balloon processes fighting for the mutex.
>
> Is this state variable necessary? balloon_process() is a work item so
> there should only be one instance of it running anyway, yes?

Hmm, yes. I've been following the comment above balloon_process()
which suggested the possibility of multiple active instances.

I'll send another version of the patch with an updated comment and
removed state variable.


Juergen