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
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 @@
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;
@@ -262,10 +268,7 @@
while (!kthread_should_stop()) {
xenfb_handle_resize_dpy(info);
- if (info->dirty) {
- info->dirty = 0;
- xenfb_update_screen(info);
- }
+ xenfb_update_screen(info);
wait_event_interruptible(info->wq,
kthread_should_stop() || info->dirty);
try_to_freeze();
@@ -666,15 +669,6 @@
if (ret < 0)
goto error;
- /* FIXME should this be delayed until backend XenbusStateConnected? */
- info->kthread = kthread_run(xenfb_thread, info, "xenfb thread");
- if (IS_ERR(info->kthread)) {
- ret = PTR_ERR(info->kthread);
- info->kthread = NULL;
- xenbus_dev_fatal(dev, ret, "register_framebuffer");
- goto error;
- }
-
return 0;
error_nomem:
@@ -839,6 +833,13 @@
"feature-resize", "%d", &val) < 0)
val = 0;
info->feature_resize = val;
+
+ info->kthread = kthread_run(xenfb_thread, info, "xenfb thread");
+ if (IS_ERR(info->kthread)) {
+ info->kthread = NULL;
+ xenbus_dev_fatal(dev, PTR_ERR(info->kthread),
+ "register_framebuffer");
+ }
break;
case XenbusStateClosing:
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home