Hi Jeremy,
The following changes since commit 384a55c0ac12eabac9531ff3a433598d32a42a15:
Jeremy Fitzhardinge (1):
Merge branch ''xen-tip/core'' into xen-tip/master
are available in the git repository at:
git://xenbits.xensource.com/people/ianc/linux-2.6.git for-jeremy/blkfront
Ian Campbell (2):
xen/blkfront: fix warning when deleting gendisk on unplug/shutdown.
xen/blkfront: allow xenbus state transition to Closing->Closed when not
Connected.
drivers/block/xen-blkfront.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2009-May-18 12:49 UTC
[Xen-devel] [PATCH] xen/blkfront: fix warning when deleting gendisk on unplug/shutdown.
Currently blkfront gives a warning when hot unplugging due to calling del_gendisk() with interrupts disabled (due to blkif_io_lock). WARNING: at kernel/softirq.c:124 local_bh_enable+0x36/0x84() Modules linked in: xenfs xen_netfront ext3 jbd mbcache xen_blkfront Pid: 13, comm: xenwatch Not tainted 2.6.29-xs5.5.0.13 #3 Call Trace: [<c012611c>] warn_slowpath+0x80/0xb6 [<c0104cf1>] xen_sched_clock+0x16/0x63 [<c0104710>] xen_force_evtchn_callback+0xc/0x10 [<c0104e32>] check_events+0x8/0xe [<c0104d9b>] xen_restore_fl_direct_end+0x0/0x1 [<c0103749>] xen_mc_flush+0x10a/0x13f [<c0105bd2>] __switch_to+0x114/0x14e [<c011d92b>] dequeue_task+0x62/0x70 [<c0123b6f>] finish_task_switch+0x2b/0x84 [<c0299877>] schedule+0x66d/0x6e7 [<c0104710>] xen_force_evtchn_callback+0xc/0x10 [<c0104710>] xen_force_evtchn_callback+0xc/0x10 [<c012a642>] local_bh_enable+0x36/0x84 [<c022f9a7>] sk_filter+0x57/0x5c [<c0233dae>] netlink_broadcast+0x1d5/0x315 [<c01c6371>] kobject_uevent_env+0x28d/0x331 [<c01e7ead>] device_del+0x10f/0x120 [<c01e7ec6>] device_unregister+0x8/0x10 [<c015f86d>] bdi_unregister+0x2d/0x39 [<c01bf6f4>] unlink_gendisk+0x23/0x3e [<c01ac946>] del_gendisk+0x7b/0xe7 [<d0828c19>] blkfront_closing+0x28/0x6e [xen_blkfront] [<d082900c>] backend_changed+0x3ad/0x41d [xen_blkfront] We can fix this by calling del_gendisk() later in blkfront_closing, after releasing blkif_io_lock. Since the queue is stopped during the interrupts disabled phase I don''t think there is any danger of an event occuring between releasing the blkif_io_lock and deleting the disk. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Jeremy Fitzhardinge <jeremy@goop.org> --- drivers/block/xen-blkfront.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 8f90508..aa0c94b 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -934,8 +934,6 @@ static void blkfront_closing(struct xenbus_device *dev) spin_lock_irqsave(&blkif_io_lock, flags); - del_gendisk(info->gd); - /* No more blkif_request(). */ blk_stop_queue(info->rq); @@ -949,6 +947,8 @@ static void blkfront_closing(struct xenbus_device *dev) blk_cleanup_queue(info->rq); info->rq = NULL; + del_gendisk(info->gd); + out: xenbus_frontend_closed(dev); } -- 1.5.6.5 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2009-May-18 12:49 UTC
[Xen-devel] [PATCH] xen/blkfront: allow xenbus state transition to Closing->Closed when not Connected.
This situation can occur when attempting to attach a block device whose backend
is an empty physical CD-ROM driver. The backend in this case will go directly
from the Initialising state to Closing->Closed. Previously this would result
in
a NULL pointer deref on info->gd (xenbus_dev_fatal does not return as
a1a15ac5
seems to expect)
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
---
drivers/block/xen-blkfront.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index aa0c94b..a6cbf7b 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -977,8 +977,10 @@ static void backend_changed(struct xenbus_device *dev,
break;
case XenbusStateClosing:
- if (info->gd == NULL)
- xenbus_dev_fatal(dev, -ENODEV, "gd is NULL");
+ if (info->gd == NULL) {
+ xenbus_frontend_closed(dev);
+ break;
+ }
bd = bdget_disk(info->gd, 0);
if (bd == NULL)
xenbus_dev_fatal(dev, -ENODEV, "bdget failed");
--
1.5.6.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2009-May-18 17:23 UTC
[Xen-devel] Re: [PATCH] xen/blkfront: allow xenbus state transition to Closing->Closed when not Connected.
Ian Campbell wrote:> This situation can occur when attempting to attach a block device whose backend > is an empty physical CD-ROM driver. The backend in this case will go directly > from the Initialising state to Closing->Closed. Previously this would result in > a NULL pointer deref on info->gd (xenbus_dev_fatal does not return as a1a15ac5 > seems to expect) > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Cc: Jeremy Fitzhardinge <jeremy@goop.org> >Jens, does this look OK to you? Acked-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com> Thanks, J> --- > drivers/block/xen-blkfront.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c > index aa0c94b..a6cbf7b 100644 > --- a/drivers/block/xen-blkfront.c > +++ b/drivers/block/xen-blkfront.c > @@ -977,8 +977,10 @@ static void backend_changed(struct xenbus_device *dev, > break; > > case XenbusStateClosing: > - if (info->gd == NULL) > - xenbus_dev_fatal(dev, -ENODEV, "gd is NULL"); > + if (info->gd == NULL) { > + xenbus_frontend_closed(dev); > + break; > + } > bd = bdget_disk(info->gd, 0); > if (bd == NULL) > xenbus_dev_fatal(dev, -ENODEV, "bdget failed"); >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jens Axboe
2009-May-18 18:34 UTC
[Xen-devel] Re: [PATCH] xen/blkfront: allow xenbus state transition to Closing->Closed when not Connected.
On Mon, May 18 2009, Jeremy Fitzhardinge wrote:> Ian Campbell wrote: >> This situation can occur when attempting to attach a block device whose backend >> is an empty physical CD-ROM driver. The backend in this case will go directly >> from the Initialising state to Closing->Closed. Previously this would result in >> a NULL pointer deref on info->gd (xenbus_dev_fatal does not return as a1a15ac5 >> seems to expect) >> >> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> >> Cc: Jeremy Fitzhardinge <jeremy@goop.org> >> > > Jens, does this look OK to you? > > Acked-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>Looks fine, shall I add it for 2.6.30? Does it need a stable backport?>> drivers/block/xen-blkfront.c | 6 ++++-- >> 1 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c >> index aa0c94b..a6cbf7b 100644 >> --- a/drivers/block/xen-blkfront.c >> +++ b/drivers/block/xen-blkfront.c >> @@ -977,8 +977,10 @@ static void backend_changed(struct xenbus_device *dev, >> break; >> case XenbusStateClosing: >> - if (info->gd == NULL) >> - xenbus_dev_fatal(dev, -ENODEV, "gd is NULL"); >> + if (info->gd == NULL) { >> + xenbus_frontend_closed(dev); >> + break; >> + } >> bd = bdget_disk(info->gd, 0); >> if (bd == NULL) >> xenbus_dev_fatal(dev, -ENODEV, "bdget failed"); >> >-- Jens Axboe _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2009-May-18 18:41 UTC
[Xen-devel] Re: [PATCH] xen/blkfront: allow xenbus state transition to Closing->Closed when not Connected.
Jens Axboe wrote:> On Mon, May 18 2009, Jeremy Fitzhardinge wrote: > >> Ian Campbell wrote: >> >>> This situation can occur when attempting to attach a block device whose backend >>> is an empty physical CD-ROM driver. The backend in this case will go directly >>> from the Initialising state to Closing->Closed. Previously this would result in >>> a NULL pointer deref on info->gd (xenbus_dev_fatal does not return as a1a15ac5 >>> seems to expect) >>> >>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> >>> Cc: Jeremy Fitzhardinge <jeremy@goop.org> >>> >>> >> Jens, does this look OK to you? >> >> Acked-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com> >> > > Looks fine, shall I add it for 2.6.30? Does it need a stable backport? >Yes please, and for .29. As far as I know there''s been no change to blkfront, so it should apply as-is. There''s another patch to go with it that I''m just having a closer look at. I''ll probably send it your way shortly. Thanks, J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2009-May-18 19:11 UTC
Re: [Xen-devel] [PATCH] xen/blkfront: fix warning when deleting gendisk on unplug/shutdown.
Ian Campbell wrote:> Currently blkfront gives a warning when hot unplugging due to calling > del_gendisk() with interrupts disabled (due to blkif_io_lock). > > WARNING: at kernel/softirq.c:124 local_bh_enable+0x36/0x84() > Modules linked in: xenfs xen_netfront ext3 jbd mbcache xen_blkfront > Pid: 13, comm: xenwatch Not tainted 2.6.29-xs5.5.0.13 #3 > Call Trace: > [<c012611c>] warn_slowpath+0x80/0xb6 > [<c0104cf1>] xen_sched_clock+0x16/0x63 > [<c0104710>] xen_force_evtchn_callback+0xc/0x10 > [<c0104e32>] check_events+0x8/0xe > [<c0104d9b>] xen_restore_fl_direct_end+0x0/0x1 > [<c0103749>] xen_mc_flush+0x10a/0x13f > [<c0105bd2>] __switch_to+0x114/0x14e > [<c011d92b>] dequeue_task+0x62/0x70 > [<c0123b6f>] finish_task_switch+0x2b/0x84 > [<c0299877>] schedule+0x66d/0x6e7 > [<c0104710>] xen_force_evtchn_callback+0xc/0x10 > [<c0104710>] xen_force_evtchn_callback+0xc/0x10 > [<c012a642>] local_bh_enable+0x36/0x84 > [<c022f9a7>] sk_filter+0x57/0x5c > [<c0233dae>] netlink_broadcast+0x1d5/0x315 > [<c01c6371>] kobject_uevent_env+0x28d/0x331 > [<c01e7ead>] device_del+0x10f/0x120 > [<c01e7ec6>] device_unregister+0x8/0x10 > [<c015f86d>] bdi_unregister+0x2d/0x39 > [<c01bf6f4>] unlink_gendisk+0x23/0x3e > [<c01ac946>] del_gendisk+0x7b/0xe7 > [<d0828c19>] blkfront_closing+0x28/0x6e [xen_blkfront] > [<d082900c>] backend_changed+0x3ad/0x41d [xen_blkfront] > > We can fix this by calling del_gendisk() later in blkfront_closing, after > releasing blkif_io_lock. Since the queue is stopped during the interrupts > disabled phase I don''t think there is any danger of an event occuring between > releasing the blkif_io_lock and deleting the disk. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Cc: Jeremy Fitzhardinge <jeremy@goop.org> >Looks OK to me. Alex Nixon came up with more or less the same patch to replace his first failed attempt to fix this. It looks much better than mucking around with interrupts states. Jens? How does it look to you? Thanks, J> --- > drivers/block/xen-blkfront.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c > index 8f90508..aa0c94b 100644 > --- a/drivers/block/xen-blkfront.c > +++ b/drivers/block/xen-blkfront.c > @@ -934,8 +934,6 @@ static void blkfront_closing(struct xenbus_device *dev) > > spin_lock_irqsave(&blkif_io_lock, flags); > > - del_gendisk(info->gd); > - > /* No more blkif_request(). */ > blk_stop_queue(info->rq); > > @@ -949,6 +947,8 @@ static void blkfront_closing(struct xenbus_device *dev) > blk_cleanup_queue(info->rq); > info->rq = NULL; > > + del_gendisk(info->gd); > + > out: > xenbus_frontend_closed(dev); > } >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jens Axboe
2009-May-19 06:28 UTC
Re: [Xen-devel] [PATCH] xen/blkfront: fix warning when deleting gendisk on unplug/shutdown.
On Mon, May 18 2009, Jeremy Fitzhardinge wrote:> Ian Campbell wrote: >> Currently blkfront gives a warning when hot unplugging due to calling >> del_gendisk() with interrupts disabled (due to blkif_io_lock). >> >> WARNING: at kernel/softirq.c:124 local_bh_enable+0x36/0x84() >> Modules linked in: xenfs xen_netfront ext3 jbd mbcache xen_blkfront >> Pid: 13, comm: xenwatch Not tainted 2.6.29-xs5.5.0.13 #3 >> Call Trace: >> [<c012611c>] warn_slowpath+0x80/0xb6 >> [<c0104cf1>] xen_sched_clock+0x16/0x63 >> [<c0104710>] xen_force_evtchn_callback+0xc/0x10 >> [<c0104e32>] check_events+0x8/0xe >> [<c0104d9b>] xen_restore_fl_direct_end+0x0/0x1 >> [<c0103749>] xen_mc_flush+0x10a/0x13f >> [<c0105bd2>] __switch_to+0x114/0x14e >> [<c011d92b>] dequeue_task+0x62/0x70 >> [<c0123b6f>] finish_task_switch+0x2b/0x84 >> [<c0299877>] schedule+0x66d/0x6e7 >> [<c0104710>] xen_force_evtchn_callback+0xc/0x10 >> [<c0104710>] xen_force_evtchn_callback+0xc/0x10 >> [<c012a642>] local_bh_enable+0x36/0x84 >> [<c022f9a7>] sk_filter+0x57/0x5c >> [<c0233dae>] netlink_broadcast+0x1d5/0x315 >> [<c01c6371>] kobject_uevent_env+0x28d/0x331 >> [<c01e7ead>] device_del+0x10f/0x120 >> [<c01e7ec6>] device_unregister+0x8/0x10 >> [<c015f86d>] bdi_unregister+0x2d/0x39 >> [<c01bf6f4>] unlink_gendisk+0x23/0x3e >> [<c01ac946>] del_gendisk+0x7b/0xe7 >> [<d0828c19>] blkfront_closing+0x28/0x6e [xen_blkfront] >> [<d082900c>] backend_changed+0x3ad/0x41d [xen_blkfront] >> >> We can fix this by calling del_gendisk() later in blkfront_closing, after >> releasing blkif_io_lock. Since the queue is stopped during the interrupts >> disabled phase I don''t think there is any danger of an event occuring between >> releasing the blkif_io_lock and deleting the disk. >> >> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> >> Cc: Jeremy Fitzhardinge <jeremy@goop.org> >> > > Looks OK to me. Alex Nixon came up with more or less the same patch to > replace his first failed attempt to fix this. It looks much better than > mucking around with interrupts states. > > Jens? How does it look to you?Yes, I have applied this one as well. -- Jens Axboe _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel