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

[Markus Armbruster <armbru@xxxxxxxxxx>]

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