| Message ID | 1420739133-27514-5-git-send-email-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello, On Thu, Jan 08, 2015 at 06:45:25PM +0100, Christoph Hellwig wrote: > Since "bdi: reimplement bdev_inode_switch_bdi()" the block device code 018a17bdc865 ("bdi: reimplement bdev_inode_switch_bdi()") would be better. > writes out all dirty data whenever switching the backing_dev_info for > a block device inode. But a block device inode can only be dirtied > when it is in use, which means we only have to write it out on the > final blkdev_put, but not when doing a blkdev_get. > @@ -1464,9 +1469,11 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part) > WARN_ON_ONCE(bdev->bd_holders); > sync_blockdev(bdev); > kill_bdev(bdev); > - /* ->release can cause the old bdi to disappear, > - * so must switch it out first > + /* > + * ->release can cause the queue to disappaear, so flush all ^^^^^ typo > + * dirty data before. > */ > + bdev_write_inode(bdev->bd_inode); Is this an optimization or something necessary for the following changes? If latter, maybe it's a good idea to state why this is necessary in the description? Otherwise, Acked-by: Tejun Heo <tj@kernel.org> Thanks.
On Sun, Jan 11, 2015 at 12:32:09PM -0500, Tejun Heo wrote: > Is this an optimization or something necessary for the following > changes? If latter, maybe it's a good idea to state why this is > necessary in the description? Otherwise, It gets rid of a bdi reassignment, and thus makes life a lot simpler. I'll update the commit message. -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/block_dev.c b/fs/block_dev.c index b48c41b..288ba70 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -49,6 +49,17 @@ inline struct block_device *I_BDEV(struct inode *inode) } EXPORT_SYMBOL(I_BDEV); +static void bdev_write_inode(struct inode *inode) +{ + spin_lock(&inode->i_lock); + while (inode->i_state & I_DIRTY) { + spin_unlock(&inode->i_lock); + WARN_ON_ONCE(write_inode_now(inode, true)); + spin_lock(&inode->i_lock); + } + spin_unlock(&inode->i_lock); +} + /* * Move the inode from its current bdi to a new bdi. Make sure the inode * is clean before moving so that it doesn't linger on the old bdi. @@ -56,16 +67,10 @@ EXPORT_SYMBOL(I_BDEV); static void bdev_inode_switch_bdi(struct inode *inode, struct backing_dev_info *dst) { - while (true) { - spin_lock(&inode->i_lock); - if (!(inode->i_state & I_DIRTY)) { - inode->i_data.backing_dev_info = dst; - spin_unlock(&inode->i_lock); - return; - } - spin_unlock(&inode->i_lock); - WARN_ON_ONCE(write_inode_now(inode, true)); - } + spin_lock(&inode->i_lock); + WARN_ON_ONCE(inode->i_state & I_DIRTY); + inode->i_data.backing_dev_info = dst; + spin_unlock(&inode->i_lock); } /* Kill _all_ buffers and pagecache , dirty or not.. */ @@ -1464,9 +1469,11 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part) WARN_ON_ONCE(bdev->bd_holders); sync_blockdev(bdev); kill_bdev(bdev); - /* ->release can cause the old bdi to disappear, - * so must switch it out first + /* + * ->release can cause the queue to disappaear, so flush all + * dirty data before. */ + bdev_write_inode(bdev->bd_inode); bdev_inode_switch_bdi(bdev->bd_inode, &default_backing_dev_info); }
Since "bdi: reimplement bdev_inode_switch_bdi()" the block device code writes out all dirty data whenever switching the backing_dev_info for a block device inode. But a block device inode can only be dirtied when it is in use, which means we only have to write it out on the final blkdev_put, but not when doing a blkdev_get. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/block_dev.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-)