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@jp.fujitsu.com>
Best Regards,
Akio Takebe
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Jan Beulich
2009-Jan-27 08:16 UTC
Re: [Xen-devel] [Patch] fix xenfb_update_screen bogus rect
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@jp.fujitsu.com> 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@jp.fujitsu.com> Best Regards, Akio Takebe _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Akio Takebe
2009-Jan-28 07:31 UTC
Re: [Xen-devel] [Patch] fix xenfb_update_screen bogus rect
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@jp.fujitsu.com> 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@jp.fujitsu.com> 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@jp.fujitsu.com> > > Best Regards, > > Akio Takebe > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2009-Jan-28 09:06 UTC
Re: [Xen-devel] [Patch] fix xenfb_update_screen bogus rect
Generally this looks okay (assuming it works), but couldn''t the update_wanted field then go away altogether? Jan>>> Akio Takebe <takebe_akio@jp.fujitsu.com> 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@jp.fujitsu.com> 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@jp.fujitsu.com> 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@jp.fujitsu.com> > > Best Regards, > > Akio Takebe > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Markus Armbruster
2009-Jan-28 14:31 UTC
Re: [Xen-devel] [Patch] fix xenfb_update_screen bogus rect
Akio Takebe <takebe_akio@jp.fujitsu.com> 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@jp.fujitsu.com> > > 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@lists.xensource.com http://lists.xensource.com/xen-devel
Markus Armbruster
2009-Jan-28 14:33 UTC
Re: [Xen-devel] [Patch] fix xenfb_update_screen bogus rect
"Jan Beulich" <jbeulich@novell.com> writes:> Generally this looks okay (assuming it works), but couldn''t the update_wanted > field then go away altogether? > > JanI think so, yes. Start the kthread only when request-update. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Akio Takebe
2009-Jan-29 09:12 UTC
Re: [Xen-devel] [Patch] fix xenfb_update_screen bogus rect
Hi, Markus and Jan Markus Armbruster wrote:> "Jan Beulich" <jbeulich@novell.com> writes: > >> Generally this looks okay (assuming it works), but couldn''t the update_wanted >> field then go away altogether? >> >> Jan > > I think so, yes. Start the kthread only when request-update. >Thank you for your review. The patch was commited already. So I made an additional patch which eliminate the update_wanted field. Signed-off-by: Akio Takebe <takebe_akio@jp.fujitsu.com> Best Regards, Akio Takebe _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Akio Takebe
2009-Jan-29 12:54 UTC
Re: [Xen-devel] [Patch] fix xenfb_update_screen bogus rect
Hi, Markus Markus Armbruster wrote:> Akio Takebe <takebe_akio@jp.fujitsu.com> 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@jp.fujitsu.com> >> >> 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. >Thank you for your review. First, I thought mm_lock just protected the mappings, and the dirty rectangle was protected by the dirty_lock. So I moved the mm_lock. Also when I tested the patch, I didn''t get any problem. Do you mean the narrow point of between unloking dirty_lock and locking mm_lock in xenfb_update_screen() may be not safe? Do you find any critical cases? Best Regards, Akio Takebe _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Markus Armbruster
2009-Jan-29 14:29 UTC
Re: [Xen-devel] [Patch] fix xenfb_update_screen bogus rect
Akio Takebe <takebe_akio@jp.fujitsu.com> writes:> Hi, Markus and Jan > > Markus Armbruster wrote: >> "Jan Beulich" <jbeulich@novell.com> writes: >> >>> Generally this looks okay (assuming it works), but couldn''t the update_wanted >>> field then go away altogether? >>> >>> Jan >> >> I think so, yes. Start the kthread only when request-update. >> > Thank you for your review. > The patch was commited already. > So I made an additional patch which eliminate the update_wanted field. > > Signed-off-by: Akio Takebe <takebe_akio@jp.fujitsu.com> > > Best Regards, > > Akio TakebeLooks good to me. A possible reason not to take this cleanup is the fact that this version of the driver is a dead end (the living end lives in current upstream kernels). Perhaps only true fixes with minimal structural impact are desired because of that. Not my call to make. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Markus Armbruster
2009-Jan-29 14:44 UTC
Re: [Xen-devel] [Patch] fix xenfb_update_screen bogus rect
Akio Takebe <takebe_akio@jp.fujitsu.com> writes:> Hi, Markus > > Markus Armbruster wrote: >> Akio Takebe <takebe_akio@jp.fujitsu.com> writes:[...]>>> 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. >> > Thank you for your review.My pleasure. I have to thank for your fix!> First, I thought mm_lock just protected the mappings, > and the dirty rectangle was protected by the dirty_lock. > So I moved the mm_lock. > Also when I tested the patch, I didn''t get any problem.Subtle locking bugs are notoriously hard to demonstrate by testing.> Do you mean the narrow point of between unloking dirty_lock and locking mm_lock > in xenfb_update_screen() may be not safe? > Do you find any critical cases? > > Best Regards, > > Akio TakebeYour proposed change might be fine, it might be racy, I don''t know. What I do know is that it invalidates the comment I mentioned. That''s a nasty trap for the unwary, and clearly a bug as far as I''m concerned. I strongly recommend to either revert the locking change, or fix the comment to match the new code. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Akio Takebe
2009-Jan-30 02:02 UTC
Re: [Xen-devel] [Patch] fix xenfb_update_screen bogus rect
Hi, Markus Markus Armbruster wrote:> Akio Takebe <takebe_akio@jp.fujitsu.com> writes: > >> Hi, Markus >> >> Markus Armbruster wrote: >>> Akio Takebe <takebe_akio@jp.fujitsu.com> writes: > [...] >>>> 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. >>> >> Thank you for your review. > > My pleasure. I have to thank for your fix! > >> First, I thought mm_lock just protected the mappings, >> and the dirty rectangle was protected by the dirty_lock. >> So I moved the mm_lock. >> Also when I tested the patch, I didn''t get any problem. > > Subtle locking bugs are notoriously hard to demonstrate by testing. > >> Do you mean the narrow point of between unloking dirty_lock and locking mm_lock >> in xenfb_update_screen() may be not safe? >> Do you find any critical cases? >> >> Best Regards, >> >> Akio Takebe > > Your proposed change might be fine, it might be racy, I don''t know. > What I do know is that it invalidates the comment I mentioned. That''s > a nasty trap for the unwary, and clearly a bug as far as I''m > concerned. > > I strongly recommend to either revert the locking change, or fix the > comment to match the new code. >I agree with you. I revert the mm_lock changing. In my graphic test, I didn''t get any problem with this patch. Signed-off-by: Akio Takebe <takebe_akio@jp.fujitsu.com> Best Regards, Akio Takebe _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Markus Armbruster
2009-Jan-30 07:38 UTC
Re: [Xen-devel] [Patch] fix xenfb_update_screen bogus rect
Akio Takebe <takebe_akio@jp.fujitsu.com> writes:> Hi, Markus > > Markus Armbruster wrote:[...]>> I strongly recommend to either revert the locking change, or fix the >> comment to match the new code. >> > I agree with you. I revert the mm_lock changing. > In my graphic test, I didn''t get any problem with this patch. > > Signed-off-by: Akio Takebe <takebe_akio@jp.fujitsu.com> > > Best Regards, > > Akio TakebeLooks good to me. Thanks again! _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Markus Armbruster
2009-Feb-02 09:17 UTC
[Xen-devel] [PATCH] fbfront: Improve diagnostics when kthread_run() fails
Failure is reported with xenbus_dev_fatal(..."register_framebuffer"),
which was already suboptimal before it got moved away from
register_framebuffer(), and is outright misleading now.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
diff -r 26ddc59c674d drivers/xen/fbfront/xenfb.c
--- a/drivers/xen/fbfront/xenfb.c Fri Jan 30 10:54:10 2009 +0000
+++ b/drivers/xen/fbfront/xenfb.c Mon Feb 02 10:12:54 2009 +0100
@@ -211,7 +211,7 @@
return;
mutex_lock(&info->mm_lock);
-
+
spin_lock_irqsave(&info->dirty_lock, flags);
if (info->dirty){
info->dirty = 0;
@@ -837,7 +837,7 @@
if (IS_ERR(info->kthread)) {
info->kthread = NULL;
xenbus_dev_fatal(dev, PTR_ERR(info->kthread),
- "register_framebuffer");
+ "xenfb_thread");
}
}
break;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Akio Takebe
2009-Feb-02 23:48 UTC
Re: [Xen-devel] [PATCH] fbfront: Improve diagnostics when kthread_run()fails
Hi, Markus Thanks. I agree with you. Best Regards, Akio Takebe>Failure is reported with xenbus_dev_fatal(..."register_framebuffer"), >which was already suboptimal before it got moved away from >register_framebuffer(), and is outright misleading now. > >Signed-off-by: Markus Armbruster <armbru@redhat.com> > >diff -r 26ddc59c674d drivers/xen/fbfront/xenfb.c >--- a/drivers/xen/fbfront/xenfb.c Fri Jan 30 10:54:10 2009 +0000 >+++ b/drivers/xen/fbfront/xenfb.c Mon Feb 02 10:12:54 2009 +0100 >@@ -211,7 +211,7 @@ > return; > > mutex_lock(&info->mm_lock); >- >+ > spin_lock_irqsave(&info->dirty_lock, flags); > if (info->dirty){ > info->dirty = 0; >@@ -837,7 +837,7 @@ > if (IS_ERR(info->kthread)) { > info->kthread = NULL; > xenbus_dev_fatal(dev, PTR_ERR(info->kthread), >- "register_framebuffer"); >+ "xenfb_thread"); > } > } > break; > >_______________________________________________ >Xen-devel mailing list >Xen-devel@lists.xensource.com >http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel