WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [Patch] fix xenfb_update_screen bogus rect

To: Akio Takebe <takebe_akio@xxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [Patch] fix xenfb_update_screen bogus rect
From: Markus Armbruster <armbru@xxxxxxxxxx>
Date: Wed, 28 Jan 2009 15:31:38 +0100
Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 28 Jan 2009 06:32:06 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <498009CC.1000101@xxxxxxxxxxxxxx> (Akio Takebe's message of "Wed\, 28 Jan 2009 16\:31\:24 +0900")
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <497E784B.7060003@xxxxxxxxxxxxxx> <497ED10A.76E4.0078.0@xxxxxxxxxx> <498009CC.1000101@xxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux)
Akio Takebe <takebe_akio@xxxxxxxxxxxxxx> writes:

> Hi, Jan
>
> Thank you for your review.
> I remade it simpler.
> But Just moving check-and-clear dirty flag to xenfb_update_screen(),
> a guest with vcpu=1 could not boot.
> It is caused by info->update_wanted = 0.

Thanks for debugging this!

> So I moved kthread_run() into XenbusStateConnected handling.
> How about it?
>
> Signed-off-by: Akio Takebe <takebe_akio@xxxxxxxxxxxxxx>
>
> Best Regards,
>
> Akio Takebe
[...]
> diff -r 83b71f4b5cb2 drivers/xen/fbfront/xenfb.c
> --- a/drivers/xen/fbfront/xenfb.c     Tue Jan 20 13:28:35 2009 +0000
> +++ b/drivers/xen/fbfront/xenfb.c     Thu Jan 29 01:24:06 2009 +0900
> @@ -213,17 +213,23 @@

Please use -p with diff.

>       if (xenfb_queue_full(info))
>               return;
>  
> -     mutex_lock(&info->mm_lock);
> -
>       spin_lock_irqsave(&info->dirty_lock, flags);
> -     y1 = info->y1;
> -     y2 = info->y2;
> -     x1 = info->x1;
> -     x2 = info->x2;
> -     info->x1 = info->y1 = INT_MAX;
> -     info->x2 = info->y2 = 0;
> +     if (info->dirty){
> +             info->dirty = 0;
> +             y1 = info->y1;
> +             y2 = info->y2;
> +             x1 = info->x1;
> +             x2 = info->x2;
> +             info->x1 = info->y1 = INT_MAX;
> +             info->x2 = info->y2 = 0;
> +     } else {
> +             spin_unlock_irqrestore(&info->dirty_lock, flags);
> +             return;
> +     }
>       spin_unlock_irqrestore(&info->dirty_lock, flags);
>  
> +     mutex_lock(&info->mm_lock);
> +     
>       list_for_each_entry(map, &info->mappings, link) {
>               if (!map->faults)
>                       continue;

Careful, locking is rather delicate here.  Please read the big comment
"There are three locks:", then explain why moving the locking of
mm_lock is safe.

[...]

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel