The ''git blame'' history shows that, the old transaction commit code has to do twice to ensure roots are updated and we have to flush metadata and super block manually, however, right now all of these can be handled well inside the transaction commit code without extra efforts. And the error handling part remains same with the current code, -- ''return to caller once we get error''. This saves us a transaction commit and a flush of super block, which are both heavy operations according to ftrace output analysis. Signed-off-by: Liu Bo <bo.li.liu@oracle.com> --- fs/btrfs/disk-io.c | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 62176ad..d6728c3 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -3542,25 +3542,7 @@ int btrfs_commit_super(struct btrfs_root *root) trans = btrfs_join_transaction(root); if (IS_ERR(trans)) return PTR_ERR(trans); - ret = btrfs_commit_transaction(trans, root); - if (ret) - return ret; - /* run commit again to drop the original snapshot */ - trans = btrfs_join_transaction(root); - if (IS_ERR(trans)) - return PTR_ERR(trans); - ret = btrfs_commit_transaction(trans, root); - if (ret) - return ret; - ret = btrfs_write_and_wait_transaction(NULL, root); - if (ret) { - btrfs_error(root->fs_info, ret, - "Failed to sync btree inode to disk."); - return ret; - } - - ret = write_ctree_super(NULL, root, 0); - return ret; + return btrfs_commit_transaction(trans, root); } int close_ctree(struct btrfs_root *root) -- 1.8.2.1 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Stefan Behrens
2013-Nov-04 08:49 UTC
Re: [PATCH] Btrfs: avoid heavy operations in btrfs_commit_super
On Sun, 3 Nov 2013 23:24:34 +0800, Liu Bo wrote:> The ''git blame'' history shows that, the old transaction commit code has to do > twice to ensure roots are updated and we have to flush metadata and super block > manually, however, right now all of these can be handled well inside > the transaction commit code without extra efforts. > > And the error handling part remains same with the current code, -- ''return to > caller once we get error''. > > This saves us a transaction commit and a flush of super block, which are both > heavy operations according to ftrace output analysis. > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> > --- > fs/btrfs/disk-io.c | 20 +------------------- > 1 file changed, 1 insertion(+), 19 deletions(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 62176ad..d6728c3 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -3542,25 +3542,7 @@ int btrfs_commit_super(struct btrfs_root *root) > trans = btrfs_join_transaction(root); > if (IS_ERR(trans)) > return PTR_ERR(trans); > - ret = btrfs_commit_transaction(trans, root); > - if (ret) > - return ret; > - /* run commit again to drop the original snapshot */ > - trans = btrfs_join_transaction(root); > - if (IS_ERR(trans)) > - return PTR_ERR(trans); > - ret = btrfs_commit_transaction(trans, root); > - if (ret) > - return ret; > - ret = btrfs_write_and_wait_transaction(NULL, root); > - if (ret) { > - btrfs_error(root->fs_info, ret, > - "Failed to sync btree inode to disk."); > - return ret; > - } > - > - ret = write_ctree_super(NULL, root, 0); > - return ret; > + return btrfs_commit_transaction(trans, root); > } > > int close_ctree(struct btrfs_root *root) >fs/btrfs/disk-io.c: In function ''btrfs_commit_super'': fs/btrfs/disk-io.c:3520: warning: unused variable ''ret'' -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Liu Bo
2013-Nov-04 09:58 UTC
Re: [PATCH] Btrfs: avoid heavy operations in btrfs_commit_super
On Mon, Nov 04, 2013 at 09:49:59AM +0100, Stefan Behrens wrote:> On Sun, 3 Nov 2013 23:24:34 +0800, Liu Bo wrote: > > The ''git blame'' history shows that, the old transaction commit code has to do > > twice to ensure roots are updated and we have to flush metadata and super block > > manually, however, right now all of these can be handled well inside > > the transaction commit code without extra efforts. > > > > And the error handling part remains same with the current code, -- ''return to > > caller once we get error''. > > > > This saves us a transaction commit and a flush of super block, which are both > > heavy operations according to ftrace output analysis. > > > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> > > --- > > fs/btrfs/disk-io.c | 20 +------------------- > > 1 file changed, 1 insertion(+), 19 deletions(-) > > > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > > index 62176ad..d6728c3 100644 > > --- a/fs/btrfs/disk-io.c > > +++ b/fs/btrfs/disk-io.c > > @@ -3542,25 +3542,7 @@ int btrfs_commit_super(struct btrfs_root *root) > > trans = btrfs_join_transaction(root); > > if (IS_ERR(trans)) > > return PTR_ERR(trans); > > - ret = btrfs_commit_transaction(trans, root); > > - if (ret) > > - return ret; > > - /* run commit again to drop the original snapshot */ > > - trans = btrfs_join_transaction(root); > > - if (IS_ERR(trans)) > > - return PTR_ERR(trans); > > - ret = btrfs_commit_transaction(trans, root); > > - if (ret) > > - return ret; > > - ret = btrfs_write_and_wait_transaction(NULL, root); > > - if (ret) { > > - btrfs_error(root->fs_info, ret, > > - "Failed to sync btree inode to disk."); > > - return ret; > > - } > > - > > - ret = write_ctree_super(NULL, root, 0); > > - return ret; > > + return btrfs_commit_transaction(trans, root); > > } > > > > int close_ctree(struct btrfs_root *root) > > > > fs/btrfs/disk-io.c: In function ''btrfs_commit_super'': > fs/btrfs/disk-io.c:3520: warning: unused variable ''ret'' >Oops, sorry for that, will fix it soon. thanks, -liubo -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html