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: "Jan Beulich" <jbeulich@xxxxxxxxxx>
Date: Wed, 28 Jan 2009 09:06:56 +0000
Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 28 Jan 2009 01:06:58 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <498009CC.1000101@xxxxxxxxxxxxxx>
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
Generally this looks okay (assuming it works), but couldn't the update_wanted
field then go away altogether?

Jan

>>> Akio Takebe <takebe_akio@xxxxxxxxxxxxxx> 28.01.09 08:31 >>>
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.
So I moved kthread_run() into XenbusStateConnected handling.
How about it?

Signed-off-by: Akio Takebe <takebe_akio@xxxxxxxxxxxxxx>

Best Regards,

Akio Takebe

Jan Beulich wrote:
> This looks bogus to me. check_is_dirty() definitely isn't needed - if at all, 
> a
> memory read barrier may need to be added, but since kthread_should_stop()
> is a function call even that ought to be unnecessary.
> 
> I also think the other change is more involved than it needs to be - it'd be
> much simpler to let xenfb_update_screen() check-and-clear the dirty flag
> along with reading the other fields, and bail if the flag was clear.
> 
> Jan
> 
>>>> Akio Takebe <takebe_akio@xxxxxxxxxxxxxx> 27.01.09 03:58 >>>
> Hi,
> 
> When I tested pvfb, I got the following warnings.
> It seems to be caused by checking/setting info->dirty without dirty_lock.
> We need to check/set info->dirty safely.
> 
> xenfb_update_screen bogus rect 2147483647 0 2147483647 0
>  BUG: warning at 
> /root/linux-2.6.18-xen.hg/drivers/xen/fbfront/xenfb.c:240/xenfb_update_screen()
> 
>  Call Trace:
>   [<ffffffff8036920e>] xenfb_thread+0x19b/0x2be
>   [<ffffffff8022730a>] try_to_wake_up+0x33b/0x34c
>   [<ffffffff80225c3d>] __wake_up_common+0x3e/0x68
>   [<ffffffff80241e20>] autoremove_wake_function+0x0/0x2e
>   [<ffffffff80241a75>] keventd_create_kthread+0x0/0x61
>   [<ffffffff80369073>] xenfb_thread+0x0/0x2be
>   [<ffffffff80241a75>] keventd_create_kthread+0x0/0x61
>   [<ffffffff80241ceb>] kthread+0xd4/0x109
>   [<ffffffff8020afe0>] child_rip+0xa/0x12
>   [<ffffffff80241a75>] keventd_create_kthread+0x0/0x61
>   [<ffffffff8021477f>] xen_send_IPI_mask+0x0/0xf5
>   [<ffffffff80241c17>] kthread+0x0/0x109
>   [<ffffffff8020afd6>] child_rip+0x0/0x12
> 
> 
> FYI, when I tested it, I used the following shell scripts on PV guest.
> The above warnings occurred in /var/log/messages.
> =======================
> #!/bin/bash
> 
> while true
> do
>     date
> done
> =======================
> 
> The attached patch fixed this warnings.
> How about it?
> 
> Signed-off-by: Akio Takebe <takebe_akio@xxxxxxxxxxxxxx>
> 
> Best Regards,
> 
> Akio Takebe
> 
> 


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