blkfront pvops updates
Subsystems (3)
PATCH 1/1 blkfront pvops updates
Hi.
Here are couple of bugfixes for blkfront.
1. Bdev removal was slightly buggy. [Is nobody out there using block-detach??] Also it seems like we'be been leaking the gendisk structs for a long time.
2. Fixes most common occasions where disk open/close races against backend state switches. This one didn't come for free:
* xenbus_switch_state growing transactions again.
* New mutex in blkfront_info
* Bdev .open/.close rewritten accordingly.
Cheers,
Daniel
PATCH 1/1 xenbus: Make xenbus_switch_state transactional (again)
According to the comments, this was how it's been done years ago, but apparently took an xbt pointer from elsewhere back then. The code was removed because of consistency issues: cancellation wont't roll back the saved xbdev->state.
Still, unsolicited writes to the state field remain an issue, especially if device shutdown takes thread synchronization, and subtle races cause accidental recreation of the device node.
Fixed by reintroducing the transaction. An internal one is sufficient, so the xbdev->state value remains consistent.
Also fixes the original hack to prevent infinite recursion. Instead of bailing out on the first attempt to switch to Closing, checks call depth now.
# HG changeset patch
# User Daniel Stodden <[email protected]> # Date 1272506749 25200
# Node ID deecb3ec62a1818d368d94c36e4f32493da38e2d # Parent e6dbf213cd122e278cf04a9621363ff3ed46b423
xenbus: Make xenbus_switch_state transactional (again)
According to the comments, this was how it's been done years ago, but apparently took an xbt pointer from elsewhere back then. The code was removed because of consistency issues: cancellation wont't roll back the saved xbdev->state.
Still, unsolicited writes to the state field remain an issue, especially if device shutdown takes thread synchronization, and subtle races cause accidental recreation of the device node.
Fixed by reintroducing the transaction. An internal one is sufficient, so the xbdev->state value remains consistent.
Also fixes the original hack to prevent infinite recursion. Instead of bailing out on the first attempt to switch to Closing, checks call depth now.
drivers/xen/xenbus/xenbus_client.c +77 -35
@@ -134,6 +134,64 @@ }EXPORT_SYMBOL_GPL (xenbus_watch_pathfmt); +static void xenbus_switch_fatal (struct xenbus_device *,int ,int , +const char *, ...); + +static int +__xenbus_switch_state (struct xenbus_device *dev, +enum xenbus_state state,int depth) +{ +/* We check whether the state is currently set to the given value, and +if not, then the state is set. We don't want to unconditionally +write the given state, because we don't want to fire watches +unnecessarily. Furthermore, if the node has gone, we don't write +to it, as the device will be tearing down, and we don't want to +resurrect that directory. + +Note that, because of this cached value of our state, this +function will not take a caller's Xenstore transaction +(something it was trying to in the past) because dev->state +would not get reset if the transaction was aborted. +*/ + +struct xenbus_transaction xbt; +int current_state; +int err, abort; + +if (state == dev->state) +return 0 ; + +again: + abort =1 ; + + err =xenbus_transaction_start (&xbt); +if (err) { +xenbus_switch_fatal (dev, depth, err,"starting transaction" ); +return 0 ; + } + + err =xenbus_scanf (xbt, dev->nodename,"state" ,"%d" , ¤t_state); +if (err !=1 ) + goto abort; + + err =xenbus_printf (xbt, dev->nodename,"state" ,"%d" , state); +if (err) { +xenbus_switch_fatal (dev, depth, err,"writing new state" ); + goto abort; + } + + abort =0 ; +abort: + err =xenbus_transaction_end (xbt, abort); +if (err) { +if (err == -EAGAIN && !abort) + goto again; +xenbus_switch_fatal (dev, depth, err,"ending transaction" ); + }else + dev->state = state; + +return 0 ; +}/** * xenbus_switch_state
@@ -146,42 +204,9 @@*/ int xenbus_switch_state (struct xenbus_device *dev,enum xenbus_state state) { -/* We check whether the state is currently set to the given value, and -if not, then the state is set. We don't want to unconditionally -write the given state, because we don't want to fire watches -unnecessarily. Furthermore, if the node has gone, we don't write -to it, as the device will be tearing down, and we don't want to -resurrect that directory. +return __xenbus_switch_state (dev, state,0 ); +} -Note that, because of this cached value of our state, this function -will not work inside a Xenstore transaction (something it was -trying to in the past) because dev->state would not get reset if -the transaction was aborted. - -*/ - -int current_state; -int err; - -if (state == dev->state) -return 0 ; - - err =xenbus_scanf (XBT_NIL , dev->nodename,"state" ,"%d" , - ¤t_state); -if (err !=1 ) -return 0 ; - - err =xenbus_printf (XBT_NIL , dev->nodename,"state" ,"%d" , state); -if (err) { -if (state != XenbusStateClosing)/* Avoid looping */ -xenbus_dev_fatal (dev, err,"writing new state" ); -return err; - } - - dev->state = state; - -return 0 ; -}EXPORT_SYMBOL_GPL (xenbus_switch_state);int xenbus_frontend_closed (struct xenbus_device *dev)
@@ -285,6 +310,23 @@EXPORT_SYMBOL_GPL (xenbus_dev_fatal);/** +* Equivalent to xenbus_dev_fatal(dev, err, fmt, args), but helps +* avoiding recursion within xenbus_switch_state. +*/ +static void xenbus_switch_fatal (struct xenbus_device *dev,int depth,int err, +const char *fmt, ...) +{ +va_list ap; + +va_start (ap, fmt); +xenbus_va_dev_error (dev, err, fmt, ap); +va_end (ap); + +if (!depth) +__xenbus_switch_state (dev, XenbusStateClosing,1 ); +} + +/** * xenbus_grant_ring * @dev: xenbus device * @ring_mfn: mfn of ring to grant
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
PATCH 1/1 blkfront: Fix backtrace in del_gendisk
In reply to Daniel StoddenThe call to del_gendisk follows an non-refcounted gd->queue pointer. We release the last ref in blk_cleanup_queue. Fixed by reordering releases accordingly.
# HG changeset patch
# User Daniel Stodden <[email protected]> # Date 1272506749 25200
# Node ID 7a3a94d3c7ce264f885ef54ae7a4040aafbf99a1 # Parent deecb3ec62a1818d368d94c36e4f32493da38e2d
blkfront: Fix backtrace in del_gendisk.
The call to del_gendisk follows an non-refcounted gd->queue pointer. We release the last ref in blk_cleanup_queue. Fixed by reordering releases accordingly.
drivers/block/xen-blkfront.c +3 -3
@@ -1001,14 +1001,14 @@/* Flush gnttab callback work. Must be done with no locks held. */ flush_scheduled_work (); -blk_cleanup_queue (info->rq); - info->rq =NULL ; - minor = info->gd->first_minor; nr_minors = info->gd->minors;del_gendisk (info->gd);xlbd_release_minors (minor, nr_minors); +blk_cleanup_queue (info->rq); + info->rq =NULL ; + out:if (info->xbdev)xenbus_frontend_closed (info->xbdev);
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
PATCH 1/1 blkfront: Fix gendisk leak
In reply to Daniel Stodden# HG changeset patch
# User Daniel Stodden <[email protected]> # Date 1272506749 25200
# Node ID daa6ed550e0cccc3c755ed8057e954fdba7ac734 # Parent 7a3a94d3c7ce264f885ef54ae7a4040aafbf99a1
blkfront: Fix gendisk leak.
drivers/block/xen-blkfront.c +3 -0
@@ -1009,6 +1009,9 @@blk_cleanup_queue (info->rq); info->rq =NULL ; +put_disk (info->gd); + info->gd =NULL ; + out:if (info->xbdev)xenbus_frontend_closed (info->xbdev);
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
PATCH 1/1 blkfront: Clean up vbd release
In reply to Daniel Stodden* Current blkfront_closing is rather a xlvbd_release_gendisk. Renamed in preparation of later patches (need the name again).
* Removed the misleading comment -- this only applied to the backend switch handler, and the queue is already flushed btw.
* Break out the xenbus call, callers know better when to switch frontend state.
# HG changeset patch
# User Daniel Stodden <[email protected]> # Date 1272506749 25200
# Node ID 046e1c8ed0dd519c2a5b2f8178906394b008fb7c # Parent daa6ed550e0cccc3c755ed8057e954fdba7ac734
blkfront: Clean up vbd release.
* Current blkfront_closing is rather a xlvbd_release_gendisk. Renamed in preparation of later patches (need the name again).
* Removed the misleading comment -- this only applied to the backend switch handler, and the queue is already flushed btw.
* Break out the xenbus call, callers know better when to switch frontend state.
drivers/block/xen-blkfront.c +43 -48
@@ -533,6 +533,39 @@return err; } +static void xlvbd_release_gendisk (struct blkfront_info *info) +{ +unsigned int minor, nr_minors; +unsigned long flags; + +if (info->rq ==NULL ) +return ; + +spin_lock_irqsave (&blkif_io_lock, flags); + +/* No more blkif_request(). */ +blk_stop_queue (info->rq); + +/* No more gnttab callback work. */ +gnttab_cancel_free_callback (&info->callback); +spin_unlock_irqrestore (&blkif_io_lock, flags); + +/* Flush gnttab callback work. Must be done with no locks held. */ +flush_scheduled_work (); + +del_gendisk (info->gd); + + minor = info->gd->first_minor; + nr_minors = info->gd->minors; +xlbd_release_minors (minor, nr_minors); + +blk_cleanup_queue (info->rq); + info->rq =NULL ; + +put_disk (info->gd); + info->gd =NULL ; +} +static void kick_pending_request_queues (struct blkfront_info *info) {if (!RING_FULL (&info->ring)) {
@@ -975,49 +1008,6 @@ }/** -* Handle the change of state of the backend to Closing. We must delete our -* device-layer structures now, to ensure that writes are flushed through to -* the backend. Once is this done, we can switch to Closed in -* acknowledgement. -*/ -static void blkfront_closing (struct blkfront_info *info) -{ -unsigned int minor, nr_minors; -unsigned long flags; - - -if (info->rq ==NULL ) - goto out; - -spin_lock_irqsave (&blkif_io_lock, flags); - -/* No more blkif_request(). */ -blk_stop_queue (info->rq); - -/* No more gnttab callback work. */ -gnttab_cancel_free_callback (&info->callback); -spin_unlock_irqrestore (&blkif_io_lock, flags); - -/* Flush gnttab callback work. Must be done with no locks held. */ -flush_scheduled_work (); - - minor = info->gd->first_minor; - nr_minors = info->gd->minors; -del_gendisk (info->gd); -xlbd_release_minors (minor, nr_minors); - -blk_cleanup_queue (info->rq); - info->rq =NULL ; - -put_disk (info->gd); - info->gd =NULL ; - - out: -if (info->xbdev) -xenbus_frontend_closed (info->xbdev); -} - -/** * Callback received when the backend's state changes. */ static void blkback_changed(struct xenbus_device *dev,
@@ -1055,8 +1045,11 @@if (info->users >0 )xenbus_dev_error (dev, -EBUSY ,"Device in use; refusing to close" ); -else -blkfront_closing (info); +else { +xlvbd_release_gendisk (info); +xenbus_frontend_closed (info->xbdev); + } +mutex_unlock (&bd->bd_mutex);bdput (bd);break ;
@@ -1107,11 +1100,13 @@struct xenbus_device *dev = info->xbdev;if (!dev) { -blkfront_closing (info); +xlvbd_release_gendisk (info);kfree (info); }else if (xenbus_read_driver_state (dev->otherend) - == XenbusStateClosing && info->is_ready) -blkfront_closing (info); + == XenbusStateClosing && info->is_ready) { +xlvbd_release_gendisk (info); +xenbus_frontend_closed (dev); + } }return 0 ; }
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
PATCH 1/1 blkfront: Lock blkfront_info when closing
In reply to Daniel StoddenThe bdev .open/.release fops race against backend switches to Closing, handled by the XenBus thread.
The original code attempted to serialize block device holders and xenbus only via bd_mutex. This is insufficient, the info->bd pointer may already be stale (or null) while xenbus tries to bump up the refcount. Worst case scenario is a null ptr deref.
Protect blkfront_info with a dedicated mutex.
# HG changeset patch
# User Daniel Stodden <[email protected]> # Date 1272506749 25200
# Node ID 6a52e76a65073a521d531d875d141b1e89a38954 # Parent 046e1c8ed0dd519c2a5b2f8178906394b008fb7c
blkfront: Lock blkfront_info when closing.
The bdev .open/.release fops race against backend switches to Closing, handled by the XenBus thread.
The original code attempted to serialize block device holders and xenbus only via bd_mutex. This is insufficient, the info->bd pointer may already be stale (or null) while xenbus tries to bump up the refcount. Worst case scenario is a null ptr deref.
Protect blkfront_info with a dedicated mutex.
drivers/block/xen-blkfront.c +40 -21
@@ -76,6 +76,7 @@*/ struct blkfront_info { +struct mutex mutex;struct xenbus_device *xbdev;struct gendisk *gd;int vdevice;
@@ -803,7 +804,6 @@return err; } -/** * Entry point to this code when a new device is created. Allocate the basic * structures and the ring buffer for communication with the backend, and
@@ -835,6 +835,7 @@return -ENOMEM ; } +mutex_init (&info->mutex); info->xbdev = dev; info->vdevice = vdevice; info->connected =BLKIF_STATE_DISCONNECTED ;
@@ -950,6 +951,43 @@return err; } +static void +blkfront_closing (struct blkfront_info *info) +{ +struct xenbus_device *xbdev = info->xbdev; +struct block_device *bdev =NULL ; + +mutex_lock (&info->mutex); + +if (xbdev->state == XenbusStateClosing) { +mutex_unlock (&info->mutex); +return ; + } + +if (info->gd) + bdev =bdget_disk (info->gd,0 ); + +mutex_unlock (&info->mutex); + +if (!bdev) { +xenbus_frontend_closed (xbdev); +return ; + } + +mutex_lock (&bdev->bd_mutex); + +if (info->users) { +xenbus_dev_error (xbdev, -EBUSY , +"Device in use; refusing to close" ); +xenbus_switch_state (xbdev, XenbusStateClosing); + }else { +xlvbd_release_gendisk (info); +xenbus_frontend_closed (xbdev); + } + +mutex_unlock (&bdev->bd_mutex); +bdput (bdev); +}/* * Invoked when the backend is finally 'ready' (and has told produced
@@ -1014,7 +1052,6 @@ enum xenbus_state backend_state) {struct blkfront_info *info =dev_get_drvdata (&dev->dev); -struct block_device *bd;dev_dbg (&dev->dev,"blkfront:blkback_changed to state %d.\n" , backend_state);
@@ -1033,25 +1070,7 @@break ;case XenbusStateClosing: -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" ); - -mutex_lock (&bd->bd_mutex); -if (info->users >0 ) -xenbus_dev_error (dev, -EBUSY , -"Device in use; refusing to close" ); -else { -xlvbd_release_gendisk (info); -xenbus_frontend_closed (info->xbdev); - } - -mutex_unlock (&bd->bd_mutex); -bdput (bd); +blkfront_closing (info);break ; } }
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
PATCH 1/1 blkfront: Fix blkfront backend switch race (bdev open)
In reply to Daniel StoddenWe need not mind if users grab a late handle on a closing disk. We probably even should not. But we have to make sure it's not a dead one already
Let the bdev deal with a gendisk deleted under its feet. Takes the info mutex to decide a race against backend closing.
# HG changeset patch
# User Daniel Stodden <[email protected]> # Date 1272506749 25200
# Node ID 5e73c8d7aaf5bff5caa7d431184ffed8e837997a # Parent 6a52e76a65073a521d531d875d141b1e89a38954
blkfront: Fix blkfront backend switch race (bdev open)
We need not mind if users grab a late handle on a closing disk. We probably even should not. But we have to make sure it's not a dead one already
Let the bdev deal with a gendisk deleted under its feet. Takes the info mutex to decide a race against backend closing.
drivers/block/xen-blkfront.c +16 -5
@@ -1100,12 +1100,23 @@static int blkif_open (struct block_device *bdev,fmode_t mode) { -struct blkfront_info *info = bdev->bd_disk->private_data; +struct gendisk *disk = bdev->bd_disk; +struct blkfront_info *info = disk->private_data; +int err =0 ; -if (!info->xbdev) -return -ENODEV ; - info->users++; -return 0 ; +mutex_lock (&info->mutex); +/* +* Let the bdev deal with a gendisk deleted under its feet. +*/ +if (!info->gd) + err = -ERESTARTSYS ; + +mutex_unlock (&info->mutex); + +if (!err) + ++info->users; + +return err; }static int blkif_release (struct gendisk *disk,fmode_t mode)
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
PATCH 1/1 blkfront: Fix blkfront backend switch race (bdev release)
In reply to Daniel StoddenWe cannot read backend state within bdev operations, because it risks grabbing the state change before xenbus gets to do it.
Fixed by tracking deferral with a frontend switch to Closing. State exposure isn't strictly necessary, but the backends won't mind.
For a 'clean' deferral this seems actually a more decent protocol than raising errors.
# HG changeset patch
# User Daniel Stodden <[email protected]> # Date 1272506749 25200
# Node ID b324514f8f83ef0d8c7991e2c995400937c540ac # Parent 5e73c8d7aaf5bff5caa7d431184ffed8e837997a
blkfront: Fix blkfront backend switch race (bdev release)
We cannot read backend state within bdev operations, because it risks grabbing the state change before xenbus gets to do it.
Fixed by tracking deferral with a frontend switch to Closing. State exposure isn't strictly necessary, but the backends won't mind.
For a 'clean' deferral this seems actually a more decent protocol than raising errors.
drivers/block/xen-blkfront.c +29 -14
@@ -1122,22 +1122,37 @@static int blkif_release (struct gendisk *disk,fmode_t mode) {struct blkfront_info *info = disk->private_data; - info->users--; -if (info->users ==0 ) { -/* Check whether we have been instructed to close. We will -have ignored this request initially, as the device was -still mounted. */ -struct xenbus_device *dev = info->xbdev; +struct block_device *bdev; +struct xenbus_device *xbdev; -if (!dev) { -xlvbd_release_gendisk (info); -kfree (info); - }else if (xenbus_read_driver_state (dev->otherend) - == XenbusStateClosing && info->is_ready) { -xlvbd_release_gendisk (info); -xenbus_frontend_closed (dev); - } +if (--info->users) +return 0 ; + + bdev =bdget_disk (disk,0 ); +bdput (bdev); + +/* +* Check if we have been instructed to close. We will have +* deferred this request, because the bdev was still open. +*/ + +mutex_lock (&info->mutex); + xbdev = info->xbdev; + +if (xbdev && xbdev->state == XenbusStateClosing) { +/* pending switch to state closed */ +xlvbd_release_gendisk (info); +xenbus_frontend_closed (info->xbdev); } + +mutex_unlock (&info->mutex); + +if (!xbdev) { +/* sudden device removal */ +xlvbd_release_gendisk (info); +kfree (info); + } +return 0 ; }
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
PATCH 1/1 blkfront: Lock blockfront_info during xbdev removal
In reply to Daniel StoddenSame approach as blkfront_closing:
* Grab the bdev safely, holding the info mutex.
* Zap xbdev safely, holding the info mutex.
* Try bdev removal safely, holding bd_mutex.
# HG changeset patch
# User Daniel Stodden <[email protected]> # Date 1272506749 25200
# Node ID a781d46d987f8f8a9308412ba50bad4654a3d97c # Parent b324514f8f83ef0d8c7991e2c995400937c540ac
blkfront: Lock blockfront_info during xbdev removal.
Same approach as blkfront_closing:
* Grab the bdev safely, holding the info mutex.
* Zap xbdev safely, holding the info mutex.
* Try bdev removal safely, holding bd_mutex.
drivers/block/xen-blkfront.c +29 -6
@@ -1075,18 +1075,41 @@ } } -static int blkfront_remove (struct xenbus_device *dev) +static int blkfront_remove (struct xenbus_device *xbdev) { -struct blkfront_info *info =dev_get_drvdata (&dev->dev); +struct blkfront_info *info =dev_get_drvdata (&xbdev->dev); +struct block_device *bdev =NULL ; -dev_dbg (&dev->dev,"blkfront_remove: %s removed\n" , dev->nodename); +dev_dbg (&xbdev->dev,"%s removed" , xbdev->nodename);blkif_free (info,0 ); -if (info->users ==0 ) +mutex_lock (&info->mutex); + +if (info->gd) + bdev =bdget_disk (info->gd,0 ); + + info->xbdev =NULL ; +mutex_unlock (&info->mutex); + +if (!bdev) +return 0 ; + +/* +* The xbdev was removed before we reached the Closed +* state. See if it's safe to remove the disk. If the bdev +* isn't closed yet, then release will take care of it. +*/ + +mutex_lock (&bdev->bd_mutex); + +if (!info->users) { +xlvbd_release_gendisk (info);kfree (info); -else - info->xbdev =NULL ; + } + +mutex_unlock (&bdev->bd_mutex); +bdput (bdev);return 0 ; }
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
PATCH 1/1 blkfront: Remove obsolete info->users
In reply to Daniel StoddenThis is just bd_openers, protected by the bd_mutex.
# HG changeset patch
# User Daniel Stodden <[email protected]> # Date 1272506749 25200
# Node ID fc49ba48fc5dde4869704cc05cc3737682ca650a # Parent a781d46d987f8f8a9308412ba50bad4654a3d97c
blkfront: Remove obsolete info->users.
This is just bd_openers, protected by the bd_mutex.
drivers/block/xen-blkfront.c +5 -14
@@ -93,12 +93,6 @@unsigned long shadow_free;int feature_barrier;int is_ready; - -/** -* The number of people holding this device open. We won't allow a -* hot-unplug unless this is 0. -*/ -int users; };static DEFINE_SPINLOCK (blkif_io_lock);
@@ -976,7 +970,7 @@mutex_lock (&bdev->bd_mutex); -if (info->users) { +if (bdev->bd_openers) {xenbus_dev_error (xbdev, -EBUSY ,"Device in use; refusing to close" );xenbus_switch_state (xbdev, XenbusStateClosing);
@@ -1103,7 +1097,7 @@mutex_lock (&bdev->bd_mutex); -if (!info->users) { +if (!bdev->bd_openers) {xlvbd_release_gendisk (info);kfree (info); }
@@ -1136,9 +1130,6 @@mutex_unlock (&info->mutex); -if (!err) - ++info->users; -return err; }
@@ -1148,12 +1139,12 @@struct block_device *bdev;struct xenbus_device *xbdev; -if (--info->users) -return 0 ; - bdev =bdget_disk (disk,0 );bdput (bdev); +if (bdev->bd_openers) +return 0 ; +/* * Check if we have been instructed to close. We will have * deferred this request, because the bdev was still open.
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
PATCH 1/1 blkfront: Klog the unclean release path
In reply to Daniel Stodden# HG changeset patch
# User Daniel Stodden <[email protected]> # Date 1272506749 25200
# Node ID 47771bd9c9a1ee54efa3bf9d176c72f1dcb87b89 # Parent fc49ba48fc5dde4869704cc05cc3737682ca650a
blkfront: Klog the unclean release path.
drivers/block/xen-blkfront.c +6 -0
@@ -1097,6 +1097,10 @@mutex_lock (&bdev->bd_mutex); +dev_warn (disk_to_dev (bdev->bd_disk), +"%s was hot-unplugged with %d handles" , + xbdev->nodename, bdev->bd_openers); +if (!bdev->bd_openers) {xlvbd_release_gendisk (info);kfree (info);
@@ -1155,6 +1159,7 @@if (xbdev && xbdev->state == XenbusStateClosing) {/* pending switch to state closed */ +dev_info (disk_to_dev (bdev->bd_disk),"releasing disk" );xlvbd_release_gendisk (info);xenbus_frontend_closed (info->xbdev); }
@@ -1163,6 +1168,7 @@if (!xbdev) {/* sudden device removal */ +dev_info (disk_to_dev (bdev->bd_disk),"releasing disk" );xlvbd_release_gendisk (info);kfree (info); }
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Quoted 8 lines · Daniel Stodden
Hi.
Here are couple of bugfixes for blkfront.
1. Bdev removal was slightly buggy. [Is nobody out there using block-detach??] Also it seems like we'be been leaking the gendisk structs for a long time.
Quoted 6 lines
2. Fixes most common occasions where disk open/close races against backend state switches. This one didn't come for free:
*xenbus_switch_stategrowing transactions again.
* New mutex inblkfront_info
* Bdev .open/.close rewritten accordingly.
Thanks for this. However, when I went to apply it, it looks like I've made a mess of the various blkfront branches.
xen/frontend has some changes from Jan and Ky which are merged into xen/next and the various stable branches.
I also have xen/blkfront, which has your last set of changes, which I seem to have forgotten to merge into anything else, and now conflicts non-trivially with xen/blkfront. I'd like to add this new set of patches into xen/blkfront, but it doesn't apply cleanly.
Could you look at this and advise me what I should do?
Thanks,
J
Quoted 4 lines
Cheers,
Daniel
Quoted 22 lines · Jeremy Fitzhardinge
Quoted 8 lines · Daniel Stodden
Hi.
Here are couple of bugfixes for blkfront.
1. Bdev removal was slightly buggy. [Is nobody out there using block-detach??] Also it seems like we'be been leaking the gendisk structs for a long time.
Quoted 6 lines
2. Fixes most common occasions where disk open/close races against backend state switches. This one didn't come for free:
*xenbus_switch_stategrowing transactions again.
* New mutex inblkfront_info
* Bdev .open/.close rewritten accordingly.Thanks for this. However, when I went to apply it, it looks like I've made a mess of the various blkfront branches.
xen/frontend has some changes from Jan and Ky which are merged into xen/next and the various stable branches.
Quoted 6 lines
I also have xen/blkfront, which has your last set of changes, which I seem to have forgotten to merge into anything else, and now conflicts non-trivially with xen/blkfront. I'd like to add this new set of patches into xen/blkfront, but it doesn't apply cleanly.
Could you look at this and advise me what I should do?
Oh, NP. I'll try and see if I can gather those and what it takes to merge.
Daniel
Quoted 28 lines · Daniel Stodden
Quoted 27 lines · Jeremy Fitzhardinge
Quoted 8 lines · Daniel Stodden
Hi.
Here are couple of bugfixes for blkfront.
1. Bdev removal was slightly buggy. [Is nobody out there using block-detach??] Also it seems like we'be been leaking the gendisk structs for a long time.
Quoted 6 lines
2. Fixes most common occasions where disk open/close races against backend state switches. This one didn't come for free:
*xenbus_switch_stategrowing transactions again.
* New mutex inblkfront_info
* Bdev .open/.close rewritten accordingly.Thanks for this. However, when I went to apply it, it looks like I've made a mess of the various blkfront branches.
xen/frontend has some changes from Jan and Ky which are merged into xen/next and the various stable branches.
I also have xen/blkfront, which has your last set of changes, which I seem to have forgotten to merge into anything else, and now conflicts non-trivially with xen/blkfront. I'd like to add this new set of patches into xen/blkfront, but it doesn't apply cleanly.
Quoted 4 lines
Quoted 1 line
Could you look at this and advise me what I should do?
Oh, NP. I'll try and see if I can gather those and what it takes to merge.
Ah, that's where the previous stuff ended up. I thought those probably just got dropped.
Let's just scratch those. It's been months, so if they didn't make it back then, they won't need to complicate stuff now either.
So I should probably merge with xen/frontend. If those matter, they must be on some branch which matters. Which one is it? I thought the one which matters is xen/master? Is it xen/next?
Daniel
Quoted 3 lines · Daniel Stodden
Ah, that's where the previous stuff ended up. I thought those probably just got dropped.
I thought the changes in these patches looked somewhat familiar.
Quoted 7 lines
Let's just scratch those. It's been months, so if they didn't make it back then, they won't need to complicate stuff now either.
So I should probably merge with xen/frontend. If those matter, they must be on some branch which matters. Which one is it? I thought the one which matters is xen/master? Is it xen/next?
The flow of changes is (ideally):
<topic branch> -> xen/next -> xen/stable*
where "->" is merged into.
I'm going to clean things up a bit by re-creating xen/blkfront with the changes from xen/frontend. If you rebase your patches onto xen/frontend then I should be able to easily incorporate them into that cleanup.
Sorry for the mixup.
Thanks,
J
Quoted 15 lines · Jeremy Fitzhardinge
Quoted 3 lines · Daniel Stodden
Ah, that's where the previous stuff ended up. I thought those probably just got dropped.
I thought the changes in these patches looked somewhat familiar.
Quoted 7 lines
Let's just scratch those. It's been months, so if they didn't make it back then, they won't need to complicate stuff now either.
So I should probably merge with xen/frontend. If those matter, they must be on some branch which matters. Which one is it? I thought the one which matters is xen/master? Is it xen/next?
Quoted 5 lines
The flow of changes is (ideally):
<topic branch> -> xen/next -> xen/stable*where "->" is merged into.
So I'm probably building the wrong tree by default.
I used to make linux-2.6-pvops when starting to work. Which checks out xen/master.
How does this relate?
Daniel
Quoted 8 lines
I'm going to clean things up a bit by re-creating xen/blkfront with the changes from xen/frontend. If you rebase your patches onto xen/frontend then I should be able to easily incorporate them into that cleanup.
Sorry for the mixup.
Thanks,
J
Quoted 7 lines · Daniel Stodden
So I'm probably building the wrong tree by default.
I used to make linux-2.6-pvops when starting to work. Which checks out xen/master.
How does this relate?
xen/master is an alias for xen/stable-2.6.31.x at the moment. I just submitted a patch to make this switchover explicit, and will move it to 2.6.32 soon.
J
Quoted 6 lines · Jeremy Fitzhardinge
Quoted 5 lines · Daniel Stodden
So I'm probably building the wrong tree by default.
I used to make linux-2.6-pvops when starting to work. Which checks out xen/master.
Quoted 6 lines
Quoted 2 lines
How does this relate?
xen/master is an alias for xen/stable-2.6.31.x at the moment. I just submitted a patch to make this switchover explicit, and will move it to 2.6.32 soon.
Okay, so xen/master rather defaults to some stable tree.
If xen/frontend is the topic branch, people ideally prepare patches per topic, and trunk is rather xen/next, then the thing I still don't follow is than xen/frontend hasn't been merged since
* commit 8800a1de3df00fa994f72ad0d7c1eda9b5a0b514
| Author: Paolo Bonzini <[email protected]>
| Date: Wed Jul 8 12:27:37 2009 +0200
Why would I want to build a kernel derived from last July's state? Or, put differently: Is it okay to merge these topic branches before anything else?
Daniel
Quoted 22 lines · Daniel Stodden
Quoted 7 lines · Jeremy Fitzhardinge
Quoted 5 lines · Daniel Stodden
So I'm probably building the wrong tree by default.
I used to make linux-2.6-pvops when starting to work. Which checks out xen/master.
Quoted 7 lines
Quoted 3 lines
How does this relate?
xen/master is an alias for xen/stable-2.6.31.x at the moment. I just submitted a patch to make this switchover explicit, and will move it to 2.6.32 soon.
Okay, so xen/master rather defaults to some stable tree.
If xen/frontend is the topic branch, people ideally prepare patches per topic, and trunk is rather xen/next, then the thing I still don't follow is than xen/frontend hasn't been merged since
Quoted 8 lines
* commit
8800a1de3df00fa994f72ad0d7c1eda9b5a0b514
| Author: Paolo Bonzini <[email protected]>
| Date: Wed Jul 8 12:27:37 2009 +0200Why would I want to build a kernel derived from last July's state? Or, put differently: Is it okay to merge these topic branches before anything else?
My usual workflow is:
1. Develop some patches on xen/next, until they work
2. Rebase those patches onto a topic branch
3. Merge that topic branch onto xen/next
This is necessary because an individual topic branch isn't necessarily compilable into a working kernel (though they should always compile).
The topic branches are often based on very old kernel versions, and I avoid rebasing or merging new kernels into them for as long as possible. This is so that they will share a common base version with any other branch I want to merge them into (including mainline Linux), so that 3-way merging is most likely to work.
It gets awkward if xen/next has other changes which conflict with the topic branch either because some other piece of necessary infrastructure is missing, or just plain merge conflicts. If they're minor then I resolve them in the merge to xen/next (git rerere is your friend for
remembering these kinds of conflict resolutions). But if there's some
major subsystem change, then I'll update the topic branch by merging in an appropriate branch to update the branch (ideally always from the mainline Linux branch, or from another Xen topic branch).
J
Quoted 18 lines · Daniel Stodden
Quoted 7 lines · Jeremy Fitzhardinge
Quoted 5 lines · Daniel Stodden
So I'm probably building the wrong tree by default.
I used to make linux-2.6-pvops when starting to work. Which checks out xen/master.
Quoted 7 lines
Quoted 3 lines
How does this relate?
xen/master is an alias for xen/stable-2.6.31.x at the moment. I just submitted a patch to make this switchover explicit, and will move it to 2.6.32 soon.
Okay, so xen/master rather defaults to some stable tree.
Quoted 8 lines
If xen/frontend is the topic branch, people ideally prepare patches per topic, and trunk is rather xen/next, then the thing I still don't follow is than xen/frontend hasn't been merged since
* commit
8800a1de3df00fa994f72ad0d7c1eda9b5a0b514
| Author: Paolo Bonzini <[email protected]>
| Date: Wed Jul 8 12:27:37 2009 +0200
BTW, it looks like you haven't updated your tree in a while. That branch has a few more changes on it since that one:
ad9ecc8ebfae8cab30ea2467b987f9b18b3172cc xen/blkfront: revalidate after setting capacity 7a634554e8ba2be858448bcf9617527c34ce326b xen/blkfront: avoid compiler warning from missing cases 065157270688777b934770fa4dcfcd68800560c3 xen/front: Propagate changed size of VBDs 1ebf91c24ab17ee62afeb1640278dd02da781fd7 blkfront: don't access freed struct xenbus_device eba64a07039d46222ca9d157c8c0549f4177f53d xen: fbdev frontend needs xenbus frontend eebc80638fe365a0386ed5ee57c7f0c6d5e56fb1 blkfront: fixes for 'xm block-detach ... --force' 71133087313f15db44ffb6ea802e5bdb2479a600 xen: use less generic names in blkfront driver. 9c9c87f53f87d0368ef04207cce4c92884f4ae3d xen: use less generic names in netfront driver. 11b84f69d36b518ff862a1bcd61ace7f8dac76b5 xen: wait up to 5 minutes for device connetion ed9ef1e71122628b50b0a526607509511b0d9135 xen: improvement to wait_for_devices()
J
Quoted 31 lines · Jeremy Fitzhardinge
Quoted 18 lines · Daniel Stodden
Quoted 7 lines · Jeremy Fitzhardinge
Quoted 5 lines · Daniel Stodden
So I'm probably building the wrong tree by default.
I used to make linux-2.6-pvops when starting to work. Which checks out xen/master.
Quoted 7 lines
Quoted 3 lines
How does this relate?
xen/master is an alias for xen/stable-2.6.31.x at the moment. I just submitted a patch to make this switchover explicit, and will move it to 2.6.32 soon.
Okay, so xen/master rather defaults to some stable tree.
Quoted 8 lines
If xen/frontend is the topic branch, people ideally prepare patches per topic, and trunk is rather xen/next, then the thing I still don't follow is than xen/frontend hasn't been merged since
* commit
8800a1de3df00fa994f72ad0d7c1eda9b5a0b514
| Author: Paolo Bonzini <[email protected]>
| Date: Wed Jul 8 12:27:37 2009 +0200BTW, it looks like you haven't updated your tree in a while. That branch has a few more changes on it since that one:
Quoted 10 lines
ad9ecc8ebfae8cab30ea2467b987f9b18b3172ccxen/blkfront: revalidate after setting capacity7a634554e8ba2be858448bcf9617527c34ce326bxen/blkfront: avoid compiler warning from missing cases065157270688777b934770fa4dcfcd68800560c3xen/front: Propagate changed size of VBDs1ebf91c24ab17ee62afeb1640278dd02da781fd7blkfront: don't access freed structxenbus_deviceeba64a07039d46222ca9d157c8c0549f4177f53dxen: fbdev frontend needs xenbus frontendeebc80638fe365a0386ed5ee57c7f0c6d5e56fb1blkfront: fixes for 'xm block-detach ... --force'71133087313f15db44ffb6ea802e5bdb2479a600xen: use less generic names in blkfront driver.9c9c87f53f87d0368ef04207cce4c92884f4ae3dxen: use less generic names in netfront driver.11b84f69d36b518ff862a1bcd61ace7f8dac76b5xen: wait up to 5 minutes for device connetioned9ef1e71122628b50b0a526607509511b0d9135xen: improvement towait_for_devices()
No, I certainly got those too. I was just surprised about where that branch was rooted. Okay, so sort of a revision- vs. bare patch- management thing then. Thanks for the clarification. 143 branches just started to make sort of sense. :o)
Daniel
Quoted 28 lines · Daniel Stodden
Quoted 27 lines · Jeremy Fitzhardinge
Quoted 8 lines · Daniel Stodden
Hi.
Here are couple of bugfixes for blkfront.
1. Bdev removal was slightly buggy. [Is nobody out there using block-detach??] Also it seems like we'be been leaking the gendisk structs for a long time.
Quoted 6 lines
2. Fixes most common occasions where disk open/close races against backend state switches. This one didn't come for free:
*xenbus_switch_stategrowing transactions again.
* New mutex inblkfront_info
* Bdev .open/.close rewritten accordingly.Thanks for this. However, when I went to apply it, it looks like I've made a mess of the various blkfront branches.
xen/frontend has some changes from Jan and Ky which are merged into xen/next and the various stable branches.
I also have xen/blkfront, which has your last set of changes, which I seem to have forgotten to merge into anything else, and now conflicts non-trivially with xen/blkfront. I'd like to add this new set of patches into xen/blkfront, but it doesn't apply cleanly.
Quoted 4 lines
Quoted 1 line
Could you look at this and advise me what I should do?
Oh, NP. I'll try and see if I can gather those and what it takes to merge.
I moved on to xen/next. That still seems to apply okay.
But as usual when looking at that kind of thing a day later, I promptly found 2 more loopholes I didn't see before. %}
These should be closed now too, but I'll rather rerun this stuff tomorrow again before resubmitting.
Thanks,
Daniel