David Vrabel
2012-Sep-21 16:04 UTC
[PATCH 0/6] xen: frontend devices should handle missed backend CLOSING
The series makes all the Xen frontend drivers handle the backend transitioning to CLOSED without the frontend having previously seen the backend in the CLOSING state. Backends shouldn''t do this but some do. e.g., if the host is XenServer and the toolstack decides to do a forced shutdown of a VBD, then the blkfront may miss the CLOSING transition and the /dev/xvdX device will not be destroyed which prevents it being reused. I have seen systems that ended up in this state but it''s not clear if this was the actual cause. However, I think in general it''s a good thing to thing to improve the handling of unexpected state transitions. Konrad, I''ve split this into a patch per frontend in case each patch should go via a different maintainer. But if you''d prefer, I can roll this up into one patch to via your Xen tree. David
David Vrabel
2012-Sep-21 16:04 UTC
[PATCH 1/6] xen-netfront: handle backend CLOSED without CLOSING
From: David Vrabel <david.vrabel@citrix.com> Backend drivers shouldn''t transistion to CLOSED unless the frontend is CLOSED. If a backend does transition to CLOSED too soon then the frontend may not see the CLOSING state and will not properly shutdown. So, treat an unexpected backend CLOSED state the same as CLOSING. Signed-off-by: David Vrabel <david.vrabel@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: <netdev@vger.kernel.org> --- drivers/net/xen-netfront.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 3089990..843533a 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -1719,7 +1719,6 @@ static void netback_changed(struct xenbus_device *dev, case XenbusStateReconfiguring: case XenbusStateReconfigured: case XenbusStateUnknown: - case XenbusStateClosed: break; case XenbusStateInitWait: @@ -1734,6 +1733,10 @@ static void netback_changed(struct xenbus_device *dev, netif_notify_peers(netdev); break; + case XenbusStateClosed: + if (dev->state == XenbusStateClosed) + break; + /* Missed the backend''s CLOSING state -- fallthrough */ case XenbusStateClosing: xenbus_frontend_closed(dev); break; -- 1.7.2.5
David Vrabel
2012-Sep-21 16:04 UTC
[PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING
From: David Vrabel <david.vrabel@citrix.com> Backend drivers shouldn''t transistion to CLOSED unless the frontend is CLOSED. If a backend does transition to CLOSED too soon then the frontend may not see the CLOSING state and will not properly shutdown. So, treat an unexpected backend CLOSED state the same as CLOSING. Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- drivers/block/xen-blkfront.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 2c2d2e5..8da12a9 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -1340,13 +1340,16 @@ static void blkback_changed(struct xenbus_device *dev, case XenbusStateReconfiguring: case XenbusStateReconfigured: case XenbusStateUnknown: - case XenbusStateClosed: break; case XenbusStateConnected: blkfront_connect(info); break; + case XenbusStateClosed: + if (dev->state == XenbusStateClosed) + break; + /* Missed the backend''s Closing state -- fallthrough */ case XenbusStateClosing: blkfront_closing(info); break; -- 1.7.2.5
David Vrabel
2012-Sep-21 16:04 UTC
[PATCH 3/6] xen-pcifront: handle backend CLOSED without CLOSING
From: David Vrabel <david.vrabel@citrix.com> Backend drivers shouldn''t transistion to CLOSED unless the frontend is CLOSED. If a backend does transition to CLOSED too soon then the frontend may not see the CLOSING state and will not properly shutdown. So, treat an unexpected backend CLOSED state the same as CLOSING. Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- drivers/pci/xen-pcifront.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c index d6cc62c..b5692c3 100644 --- a/drivers/pci/xen-pcifront.c +++ b/drivers/pci/xen-pcifront.c @@ -1069,13 +1069,16 @@ static void __init_refok pcifront_backend_changed(struct xenbus_device *xdev, case XenbusStateInitialising: case XenbusStateInitWait: case XenbusStateInitialised: - case XenbusStateClosed: break; case XenbusStateConnected: pcifront_try_connect(pdev); break; + case XenbusStateClosed: + if (xdev->state == XenbusStateClosed) + break; + /* Missed the backend''s CLOSING state -- fallthrough */ case XenbusStateClosing: dev_warn(&xdev->dev, "backend going away!\n"); pcifront_try_disconnect(pdev); -- 1.7.2.5
David Vrabel
2012-Sep-21 16:04 UTC
[PATCH 4/6] xen-fbfront: handle backend CLOSED without CLOSING
From: David Vrabel <david.vrabel@citrix.com> Backend drivers shouldn''t transistion to CLOSED unless the frontend is CLOSED. If a backend does transition to CLOSED too soon then the frontend may not see the CLOSING state and will not properly shutdown. So, treat an unexpected backend CLOSED state the same as CLOSING. Signed-off-by: David Vrabel <david.vrabel@citrix.com> Cc: <linux-fbdev@vger.kernel.org> --- drivers/video/xen-fbfront.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c index b7f5173..917bb56 100644 --- a/drivers/video/xen-fbfront.c +++ b/drivers/video/xen-fbfront.c @@ -641,7 +641,6 @@ static void xenfb_backend_changed(struct xenbus_device *dev, case XenbusStateReconfiguring: case XenbusStateReconfigured: case XenbusStateUnknown: - case XenbusStateClosed: break; case XenbusStateInitWait: @@ -670,6 +669,10 @@ InitWait: info->feature_resize = val; break; + case XenbusStateClosed: + if (dev->state == XenbusStateClosed) + break; + /* Missed the backend''s CLOSING state -- fallthrough */ case XenbusStateClosing: xenbus_frontend_closed(dev); break; -- 1.7.2.5
David Vrabel
2012-Sep-21 16:04 UTC
[PATCH 5/6] xen-kbdfront: handle backend CLOSED without CLOSING
From: David Vrabel <david.vrabel@citrix.com> Backend drivers shouldn''t transistion to CLOSED unless the frontend is CLOSED. If a backend does transition to CLOSED too soon then the frontend may not see the CLOSING state and will not properly shutdown. So, treat an unexpected backend CLOSED state the same as CLOSING. Signed-off-by: David Vrabel <david.vrabel@citrix.com> Cc: <linux-input@vger.kernel.org> --- drivers/input/misc/xen-kbdfront.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c index 02ca868..6f7d990 100644 --- a/drivers/input/misc/xen-kbdfront.c +++ b/drivers/input/misc/xen-kbdfront.c @@ -311,7 +311,6 @@ static void xenkbd_backend_changed(struct xenbus_device *dev, case XenbusStateReconfiguring: case XenbusStateReconfigured: case XenbusStateUnknown: - case XenbusStateClosed: break; case XenbusStateInitWait: @@ -350,6 +349,10 @@ InitWait: break; + case XenbusStateClosed: + if (dev->state == XenbusStateClosed) + break; + /* Missed the backend''s CLOSING state -- fallthrough */ case XenbusStateClosing: xenbus_frontend_closed(dev); break; -- 1.7.2.5 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Vrabel
2012-Sep-21 16:04 UTC
[PATCH 6/6] xen/hvc: handle backend CLOSED without CLOSING
From: David Vrabel <david.vrabel@citrix.com> Backend drivers shouldn''t transistion to CLOSED unless the frontend is CLOSED. If a backend does transition to CLOSED too soon then the frontend may not see the CLOSING state and will not properly shutdown. So, treat an unexpected backend CLOSED state the same as CLOSING. Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- drivers/tty/hvc/hvc_xen.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c index 1e456dc..82901fb 100644 --- a/drivers/tty/hvc/hvc_xen.c +++ b/drivers/tty/hvc/hvc_xen.c @@ -476,7 +476,6 @@ static void xencons_backend_changed(struct xenbus_device *dev, case XenbusStateInitialising: case XenbusStateInitialised: case XenbusStateUnknown: - case XenbusStateClosed: break; case XenbusStateInitWait: @@ -486,6 +485,10 @@ static void xencons_backend_changed(struct xenbus_device *dev, xenbus_switch_state(dev, XenbusStateConnected); break; + case XenbusStateClosed: + if (dev->state == XenbusStateClosed) + break; + /* Missed the backend''s CLOSING state -- fallthrough */ case XenbusStateClosing: xenbus_frontend_closed(dev); break; -- 1.7.2.5
Konrad Rzeszutek Wilk
2012-Sep-21 17:10 UTC
Re: [PATCH 0/6] xen: frontend devices should handle missed backend CLOSING
On Fri, Sep 21, 2012 at 05:04:18PM +0100, David Vrabel wrote:> The series makes all the Xen frontend drivers handle the backend > transitioning to CLOSED without the frontend having previously seen > the backend in the CLOSING state. > > Backends shouldn''t do this but some do. e.g., if the host is > XenServer and the toolstack decides to do a forced shutdown of a VBD, > then the blkfront may miss the CLOSING transition and the /dev/xvdX > device will not be destroyed which prevents it being reused. > > I have seen systems that ended up in this state but it''s not clear if > this was the actual cause. However, I think in general it''s a good > thing to thing to improve the handling of unexpected state > transitions. > > Konrad, I''ve split this into a patch per frontend in case each patch > should go via a different maintainer. But if you''d prefer, I can roll > this up into one patch to via your Xen tree.I like the split-up. Can you repost them with the different maintainers CC (scripts/get_maintainers.pl) and with Acked-by from me on them?> > David
David Vrabel
2012-Sep-25 17:53 UTC
Re: [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING
On 21/09/12 17:04, David Vrabel wrote:> From: David Vrabel <david.vrabel@citrix.com> > > Backend drivers shouldn''t transistion to CLOSED unless the frontend is > CLOSED. If a backend does transition to CLOSED too soon then the > frontend may not see the CLOSING state and will not properly shutdown. > > So, treat an unexpected backend CLOSED state the same as CLOSING.Didn''t handle the frontend block device being mounted. Updated patch here. 8<--------------------------------- xen-blkfront: handle backend CLOSED without CLOSING Backend drivers shouldn''t transistion to CLOSED unless the frontend is CLOSED. If a backend does transition to CLOSED too soon then the frontend may not see the CLOSING state and will not properly shutdown. So, treat an unexpected backend CLOSED state the same as CLOSING. If the backend is CLOSED then the frontend can''t talk to it so go to CLOSED immediately without waiting for the block device to be closed or unmounted. Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- drivers/block/xen-blkfront.c | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 2c2d2e5..c1f5f38 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -1143,7 +1143,8 @@ static int blkfront_resume(struct xenbus_device *dev) } static void -blkfront_closing(struct blkfront_info *info) +blkfront_closing(struct blkfront_info *info, + enum xenbus_state backend_state) { struct xenbus_device *xbdev = info->xbdev; struct block_device *bdev = NULL; @@ -1167,7 +1168,8 @@ blkfront_closing(struct blkfront_info *info) mutex_lock(&bdev->bd_mutex); - if (bdev->bd_openers) { + /* If the backend is already CLOSED, close now. */ + if (bdev->bd_openers && backend_state != XenbusStateClosed) { xenbus_dev_error(xbdev, -EBUSY, "Device in use; refusing to close"); xenbus_switch_state(xbdev, XenbusStateClosing); @@ -1340,15 +1342,18 @@ static void blkback_changed(struct xenbus_device *dev, case XenbusStateReconfiguring: case XenbusStateReconfigured: case XenbusStateUnknown: - case XenbusStateClosed: break; case XenbusStateConnected: blkfront_connect(info); break; + case XenbusStateClosed: + if (dev->state == XenbusStateClosed) + break; + /* Missed the backend''s Closing state -- fallthrough */ case XenbusStateClosing: - blkfront_closing(info); + blkfront_closing(info, backend_state); break; } } -- 1.7.2.5
David Vrabel
2012-Oct-01 17:19 UTC
Re: [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING
On 25/09/12 18:53, David Vrabel wrote:> On 21/09/12 17:04, David Vrabel wrote: >> From: David Vrabel <david.vrabel@citrix.com> >> >> Backend drivers shouldn''t transistion to CLOSED unless the frontend is >> CLOSED. If a backend does transition to CLOSED too soon then the >> frontend may not see the CLOSING state and will not properly shutdown. >> >> So, treat an unexpected backend CLOSED state the same as CLOSING. > > Didn''t handle the frontend block device being mounted. Updated patch here.Konrad, can you ack this updated patch if you''re happy with it. Thanks. David> 8<--------------------------------- > xen-blkfront: handle backend CLOSED without CLOSING > > Backend drivers shouldn''t transistion to CLOSED unless the frontend is > CLOSED. If a backend does transition to CLOSED too soon then the > frontend may not see the CLOSING state and will not properly shutdown. > > So, treat an unexpected backend CLOSED state the same as CLOSING. > > If the backend is CLOSED then the frontend can''t talk to it so go to > CLOSED immediately without waiting for the block device to be closed > or unmounted. > > Signed-off-by: David Vrabel <david.vrabel@citrix.com> > --- > drivers/block/xen-blkfront.c | 13 +++++++++---- > 1 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c > index 2c2d2e5..c1f5f38 100644 > --- a/drivers/block/xen-blkfront.c > +++ b/drivers/block/xen-blkfront.c > @@ -1143,7 +1143,8 @@ static int blkfront_resume(struct xenbus_device *dev) > } > > static void > -blkfront_closing(struct blkfront_info *info) > +blkfront_closing(struct blkfront_info *info, > + enum xenbus_state backend_state) > { > struct xenbus_device *xbdev = info->xbdev; > struct block_device *bdev = NULL; > @@ -1167,7 +1168,8 @@ blkfront_closing(struct blkfront_info *info) > > mutex_lock(&bdev->bd_mutex); > > - if (bdev->bd_openers) { > + /* If the backend is already CLOSED, close now. */ > + if (bdev->bd_openers && backend_state != XenbusStateClosed) { > xenbus_dev_error(xbdev, -EBUSY, > "Device in use; refusing to close"); > xenbus_switch_state(xbdev, XenbusStateClosing); > @@ -1340,15 +1342,18 @@ static void blkback_changed(struct xenbus_device *dev, > case XenbusStateReconfiguring: > case XenbusStateReconfigured: > case XenbusStateUnknown: > - case XenbusStateClosed: > break; > > case XenbusStateConnected: > blkfront_connect(info); > break; > > + case XenbusStateClosed: > + if (dev->state == XenbusStateClosed) > + break; > + /* Missed the backend''s Closing state -- fallthrough */ > case XenbusStateClosing: > - blkfront_closing(info); > + blkfront_closing(info, backend_state); > break; > } > }
Konrad Rzeszutek Wilk
2012-Oct-02 20:02 UTC
Re: [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING
On Mon, Oct 01, 2012 at 06:19:19PM +0100, David Vrabel wrote:> On 25/09/12 18:53, David Vrabel wrote: > > On 21/09/12 17:04, David Vrabel wrote: > >> From: David Vrabel <david.vrabel@citrix.com> > >> > >> Backend drivers shouldn''t transistion to CLOSED unless the frontend is > >> CLOSED. If a backend does transition to CLOSED too soon then the > >> frontend may not see the CLOSING state and will not properly shutdown. > >> > >> So, treat an unexpected backend CLOSED state the same as CLOSING. > > > > Didn''t handle the frontend block device being mounted. Updated patch here. > > Konrad, can you ack this updated patch if you''re happy with it.Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Or should I just carry it in my for-jens-3.7 bug-fixes queue and ask Jen to pull it once rc0 is out?> > Thanks. > > David > > > 8<--------------------------------- > > xen-blkfront: handle backend CLOSED without CLOSING > > > > Backend drivers shouldn''t transistion to CLOSED unless the frontend is > > CLOSED. If a backend does transition to CLOSED too soon then the > > frontend may not see the CLOSING state and will not properly shutdown. > > > > So, treat an unexpected backend CLOSED state the same as CLOSING. > > > > If the backend is CLOSED then the frontend can''t talk to it so go to > > CLOSED immediately without waiting for the block device to be closed > > or unmounted. > > > > Signed-off-by: David Vrabel <david.vrabel@citrix.com> > > --- > > drivers/block/xen-blkfront.c | 13 +++++++++---- > > 1 files changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c > > index 2c2d2e5..c1f5f38 100644 > > --- a/drivers/block/xen-blkfront.c > > +++ b/drivers/block/xen-blkfront.c > > @@ -1143,7 +1143,8 @@ static int blkfront_resume(struct xenbus_device *dev) > > } > > > > static void > > -blkfront_closing(struct blkfront_info *info) > > +blkfront_closing(struct blkfront_info *info, > > + enum xenbus_state backend_state) > > { > > struct xenbus_device *xbdev = info->xbdev; > > struct block_device *bdev = NULL; > > @@ -1167,7 +1168,8 @@ blkfront_closing(struct blkfront_info *info) > > > > mutex_lock(&bdev->bd_mutex); > > > > - if (bdev->bd_openers) { > > + /* If the backend is already CLOSED, close now. */ > > + if (bdev->bd_openers && backend_state != XenbusStateClosed) { > > xenbus_dev_error(xbdev, -EBUSY, > > "Device in use; refusing to close"); > > xenbus_switch_state(xbdev, XenbusStateClosing); > > @@ -1340,15 +1342,18 @@ static void blkback_changed(struct xenbus_device *dev, > > case XenbusStateReconfiguring: > > case XenbusStateReconfigured: > > case XenbusStateUnknown: > > - case XenbusStateClosed: > > break; > > > > case XenbusStateConnected: > > blkfront_connect(info); > > break; > > > > + case XenbusStateClosed: > > + if (dev->state == XenbusStateClosed) > > + break; > > + /* Missed the backend''s Closing state -- fallthrough */ > > case XenbusStateClosing: > > - blkfront_closing(info); > > + blkfront_closing(info, backend_state); > > break; > > } > > }
Jan Beulich
2012-Oct-04 10:14 UTC
Re: [Xen-devel] [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING
>>> On 25.09.12 at 19:53, David Vrabel <david.vrabel@citrix.com> wrote: > @@ -1167,7 +1168,8 @@ blkfront_closing(struct blkfront_info *info) > > mutex_lock(&bdev->bd_mutex); > > - if (bdev->bd_openers) { > + /* If the backend is already CLOSED, close now. */ > + if (bdev->bd_openers && backend_state != XenbusStateClosed) { > xenbus_dev_error(xbdev, -EBUSY, > "Device in use; refusing to close"); > xenbus_switch_state(xbdev, XenbusStateClosing);This looks wrong to me on a second glance: As long as there are users of the device, I don''t think we want to go into Closed ourselves, irrespective of the backend state. Jan
David Vrabel
2012-Oct-04 10:31 UTC
Re: [Xen-devel] [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING
On 04/10/12 11:14, Jan Beulich wrote:>>>> On 25.09.12 at 19:53, David Vrabel <david.vrabel@citrix.com> wrote: >> @@ -1167,7 +1168,8 @@ blkfront_closing(struct blkfront_info *info) >> >> mutex_lock(&bdev->bd_mutex); >> >> - if (bdev->bd_openers) { >> + /* If the backend is already CLOSED, close now. */ >> + if (bdev->bd_openers && backend_state != XenbusStateClosed) { >> xenbus_dev_error(xbdev, -EBUSY, >> "Device in use; refusing to close"); >> xenbus_switch_state(xbdev, XenbusStateClosing); > > This looks wrong to me on a second glance: As long as there > are users of the device, I don''t think we want to go into Closed > ourselves, irrespective of the backend state.Any users of the frontend device are screwed either way, as the backend is gone. It seems sensible to handle this case the same as (e.g.,) a physical unplug of a USB storage device. Removing the device and forcing all outstanding I/O to fail immediately rather than lingering in the rings, going nowhere. David
David Vrabel
2012-Oct-05 11:42 UTC
Re: [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING
On 02/10/12 21:02, Konrad Rzeszutek Wilk wrote:> On Mon, Oct 01, 2012 at 06:19:19PM +0100, David Vrabel wrote: >> On 25/09/12 18:53, David Vrabel wrote: >>> On 21/09/12 17:04, David Vrabel wrote: >>>> From: David Vrabel <david.vrabel@citrix.com> >>>> >>>> Backend drivers shouldn''t transistion to CLOSED unless the frontend is >>>> CLOSED. If a backend does transition to CLOSED too soon then the >>>> frontend may not see the CLOSING state and will not properly shutdown. >>>> >>>> So, treat an unexpected backend CLOSED state the same as CLOSING. >>> >>> Didn''t handle the frontend block device being mounted. Updated patch here. >> >> Konrad, can you ack this updated patch if you''re happy with it. > > Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > Or should I just carry it in my for-jens-3.7 bug-fixes queue and ask > Jen to pull it once rc0 is out?This seems easiest, if Jan is happy with the patch. Thanks. David
Jan Beulich
2012-Oct-05 11:55 UTC
Re: [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING
>>> On 05.10.12 at 13:42, David Vrabel <david.vrabel@citrix.com> wrote: > On 02/10/12 21:02, Konrad Rzeszutek Wilk wrote: >> On Mon, Oct 01, 2012 at 06:19:19PM +0100, David Vrabel wrote: >>> On 25/09/12 18:53, David Vrabel wrote: >>>> On 21/09/12 17:04, David Vrabel wrote: >>>>> From: David Vrabel <david.vrabel@citrix.com> >>>>> >>>>> Backend drivers shouldn''t transistion to CLOSED unless the frontend is >>>>> CLOSED. If a backend does transition to CLOSED too soon then the >>>>> frontend may not see the CLOSING state and will not properly shutdown. >>>>> >>>>> So, treat an unexpected backend CLOSED state the same as CLOSING. >>>> >>>> Didn''t handle the frontend block device being mounted. Updated patch here. >>> >>> Konrad, can you ack this updated patch if you''re happy with it. >> >> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >> >> Or should I just carry it in my for-jens-3.7 bug-fixes queue and ask >> Jen to pull it once rc0 is out? > > This seems easiest, if Jan is happy with the patch.I see the point of your explanation yesterday, but am still not convinced that the early cleanup in spite of active users of the disk can''t cause any problems (like dangling pointers or NULL dereferences). Jan
David Vrabel
2012-Oct-05 15:57 UTC
Re: [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING
On 05/10/12 12:55, Jan Beulich wrote:>>>> On 05.10.12 at 13:42, David Vrabel <david.vrabel@citrix.com> wrote: >> On 02/10/12 21:02, Konrad Rzeszutek Wilk wrote: >>> On Mon, Oct 01, 2012 at 06:19:19PM +0100, David Vrabel wrote: >>>> On 25/09/12 18:53, David Vrabel wrote: >>>>> On 21/09/12 17:04, David Vrabel wrote: >>>>>> From: David Vrabel <david.vrabel@citrix.com> >>>>>> >>>>>> Backend drivers shouldn''t transistion to CLOSED unless the frontend is >>>>>> CLOSED. If a backend does transition to CLOSED too soon then the >>>>>> frontend may not see the CLOSING state and will not properly shutdown. >>>>>> >>>>>> So, treat an unexpected backend CLOSED state the same as CLOSING. >>>>> >>>>> Didn''t handle the frontend block device being mounted. Updated patch here. >>>> >>>> Konrad, can you ack this updated patch if you''re happy with it. >>> >>> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >>> >>> Or should I just carry it in my for-jens-3.7 bug-fixes queue and ask >>> Jen to pull it once rc0 is out? >> >> This seems easiest, if Jan is happy with the patch. > > I see the point of your explanation yesterday, but am still not > convinced that the early cleanup in spite of active users of the > disk can''t cause any problems (like dangling pointers or NULL > dereferences).I tested it some more and it is broken. Please apply the first version instead. blkfront needs to cancel and complete with an error requests that are are still on the ring after the backend has disconnected and it needs to complete with an error all requests submitted while disconnected. Otherwise, if there is outstanding requests or new requests then del_gendisk() blocks waiting for the I/O to complete. David
Konrad Rzeszutek Wilk
2012-Oct-09 16:30 UTC
Re: [PATCH 2/6] xen-blkfront: handle backend CLOSED without CLOSING
On Fri, Oct 05, 2012 at 04:57:36PM +0100, David Vrabel wrote:> On 05/10/12 12:55, Jan Beulich wrote: > >>>> On 05.10.12 at 13:42, David Vrabel <david.vrabel@citrix.com> wrote: > >> On 02/10/12 21:02, Konrad Rzeszutek Wilk wrote: > >>> On Mon, Oct 01, 2012 at 06:19:19PM +0100, David Vrabel wrote: > >>>> On 25/09/12 18:53, David Vrabel wrote: > >>>>> On 21/09/12 17:04, David Vrabel wrote: > >>>>>> From: David Vrabel <david.vrabel@citrix.com> > >>>>>> > >>>>>> Backend drivers shouldn''t transistion to CLOSED unless the frontend is > >>>>>> CLOSED. If a backend does transition to CLOSED too soon then the > >>>>>> frontend may not see the CLOSING state and will not properly shutdown. > >>>>>> > >>>>>> So, treat an unexpected backend CLOSED state the same as CLOSING. > >>>>> > >>>>> Didn''t handle the frontend block device being mounted. Updated patch here. > >>>> > >>>> Konrad, can you ack this updated patch if you''re happy with it. > >>> > >>> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > >>> > >>> Or should I just carry it in my for-jens-3.7 bug-fixes queue and ask > >>> Jen to pull it once rc0 is out? > >> > >> This seems easiest, if Jan is happy with the patch. > > > > I see the point of your explanation yesterday, but am still not > > convinced that the early cleanup in spite of active users of the > > disk can''t cause any problems (like dangling pointers or NULL > > dereferences). > > I tested it some more and it is broken. Please apply the first version > instead.OK, can we do this once more - can you repost the patches, but CC the invidiual maintainers? The FB and KBD need to go through Florian I think?> > blkfront needs to cancel and complete with an error requests that are > are still on the ring after the backend has disconnected and it needs to > complete with an error all requests submitted while disconnected. > Otherwise, if there is outstanding requests or new requests then > del_gendisk() blocks waiting for the I/O to complete. > > David