From: Miao Xie <miaox@cn.fujitsu.com>
To: Josef Bacik <jbacik@fusionio.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] Btrfs: cleanup arguments to extent_clear_unlock_delalloc
Date: Tue, 30 Jul 2013 12:00:22 +0800 [thread overview]
Message-ID: <51F73A56.8050105@cn.fujitsu.com> (raw)
In-Reply-To: <1375111380-19817-1-git-send-email-jbacik@fusionio.com>
On mon, 29 Jul 2013 11:23:00 -0400, Josef Bacik wrote:
> This patch removes the io_tree argument for extent_clear_unlock_delalloc since
> we always use &BTRFS_I(inode)->io_tree, and it separates out the extent tree
> operations from the page operations. This way we just pass in the extent bits
> we want to clear and then pass in the operations we want done to the pages.
> This is because I'm going to fix what extent bits we clear in some cases and
> rather than add a bunch of new flags we'll just use the actual extent bits we
> want to clear. Thanks,
>
> Signed-off-by: Josef Bacik <jbacik@fusionio.com>
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
> ---
> fs/btrfs/extent_io.c | 32 ++++-------
> fs/btrfs/extent_io.h | 21 +++----
> fs/btrfs/inode.c | 162 ++++++++++++++++++++------------------------------
> 3 files changed, 85 insertions(+), 130 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index cc685dd..deaea9c 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -1678,31 +1678,21 @@ out_failed:
> return found;
> }
>
> -int extent_clear_unlock_delalloc(struct inode *inode,
> - struct extent_io_tree *tree,
> - u64 start, u64 end, struct page *locked_page,
> - unsigned long op)
> +int extent_clear_unlock_delalloc(struct inode *inode, u64 start, u64 end,
> + struct page *locked_page,
> + unsigned long clear_bits,
> + unsigned long page_ops)
> {
> + struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
> int ret;
> struct page *pages[16];
> unsigned long index = start >> PAGE_CACHE_SHIFT;
> unsigned long end_index = end >> PAGE_CACHE_SHIFT;
> unsigned long nr_pages = end_index - index + 1;
> int i;
> - unsigned long clear_bits = 0;
> -
> - if (op & EXTENT_CLEAR_UNLOCK)
> - clear_bits |= EXTENT_LOCKED;
> - if (op & EXTENT_CLEAR_DIRTY)
> - clear_bits |= EXTENT_DIRTY;
> -
> - if (op & EXTENT_CLEAR_DELALLOC)
> - clear_bits |= EXTENT_DELALLOC;
>
> clear_extent_bit(tree, start, end, clear_bits, 1, 0, NULL, GFP_NOFS);
> - if (!(op & (EXTENT_CLEAR_UNLOCK_PAGE | EXTENT_CLEAR_DIRTY |
> - EXTENT_SET_WRITEBACK | EXTENT_END_WRITEBACK |
> - EXTENT_SET_PRIVATE2)))
> + if (page_ops == 0)
> return 0;
>
> while (nr_pages > 0) {
> @@ -1711,20 +1701,20 @@ int extent_clear_unlock_delalloc(struct inode *inode,
> nr_pages, ARRAY_SIZE(pages)), pages);
> for (i = 0; i < ret; i++) {
>
> - if (op & EXTENT_SET_PRIVATE2)
> + if (page_ops & PAGE_SET_PRIVATE2)
> SetPagePrivate2(pages[i]);
>
> if (pages[i] == locked_page) {
> page_cache_release(pages[i]);
> continue;
> }
> - if (op & EXTENT_CLEAR_DIRTY)
> + if (page_ops & PAGE_CLEAR_DIRTY)
> clear_page_dirty_for_io(pages[i]);
> - if (op & EXTENT_SET_WRITEBACK)
> + if (page_ops & PAGE_SET_WRITEBACK)
> set_page_writeback(pages[i]);
> - if (op & EXTENT_END_WRITEBACK)
> + if (page_ops & PAGE_END_WRITEBACK)
> end_page_writeback(pages[i]);
> - if (op & EXTENT_CLEAR_UNLOCK_PAGE)
> + if (page_ops & PAGE_UNLOCK)
> unlock_page(pages[i]);
> page_cache_release(pages[i]);
> }
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index f7544af..c450620 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -44,14 +44,11 @@
> #define EXTENT_BUFFER_DUMMY 9
>
> /* these are flags for extent_clear_unlock_delalloc */
> -#define EXTENT_CLEAR_UNLOCK_PAGE 0x1
> -#define EXTENT_CLEAR_UNLOCK 0x2
> -#define EXTENT_CLEAR_DELALLOC 0x4
> -#define EXTENT_CLEAR_DIRTY 0x8
> -#define EXTENT_SET_WRITEBACK 0x10
> -#define EXTENT_END_WRITEBACK 0x20
> -#define EXTENT_SET_PRIVATE2 0x40
> -#define EXTENT_CLEAR_ACCOUNTING 0x80
> +#define PAGE_UNLOCK (1 << 0)
> +#define PAGE_CLEAR_DIRTY (1 << 1)
> +#define PAGE_SET_WRITEBACK (1 << 2)
> +#define PAGE_END_WRITEBACK (1 << 3)
> +#define PAGE_SET_PRIVATE2 (1 << 4)
>
> /*
> * page->private values. Every page that is controlled by the extent
> @@ -328,10 +325,10 @@ int map_private_extent_buffer(struct extent_buffer *eb, unsigned long offset,
> unsigned long *map_len);
> int extent_range_clear_dirty_for_io(struct inode *inode, u64 start, u64 end);
> int extent_range_redirty_for_io(struct inode *inode, u64 start, u64 end);
> -int extent_clear_unlock_delalloc(struct inode *inode,
> - struct extent_io_tree *tree,
> - u64 start, u64 end, struct page *locked_page,
> - unsigned long op);
> +int extent_clear_unlock_delalloc(struct inode *inode, u64 start, u64 end,
> + struct page *locked_page,
> + unsigned long bits_to_clear,
> + unsigned long page_ops);
> struct bio *
> btrfs_bio_alloc(struct block_device *bdev, u64 first_sector, int nr_vecs,
> gfp_t gfp_flags);
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 3f94b21..a60be02 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -489,13 +489,13 @@ cont:
> * we don't need to create any more async work items.
> * Unlock and free up our temp pages.
> */
> - extent_clear_unlock_delalloc(inode,
> - &BTRFS_I(inode)->io_tree,
> - start, end, NULL,
> - EXTENT_CLEAR_UNLOCK_PAGE | EXTENT_CLEAR_DIRTY |
> - EXTENT_CLEAR_DELALLOC |
> - EXTENT_SET_WRITEBACK | EXTENT_END_WRITEBACK);
> -
> + extent_clear_unlock_delalloc(inode, start, end, NULL,
> + EXTENT_DIRTY |
> + EXTENT_DELALLOC,
> + PAGE_UNLOCK |
> + PAGE_CLEAR_DIRTY |
> + PAGE_SET_WRITEBACK |
> + PAGE_END_WRITEBACK);
> btrfs_end_transaction(trans, root);
> goto free_pages_out;
> }
> @@ -592,13 +592,10 @@ free_pages_out:
> goto out;
>
> cleanup_and_out:
> - extent_clear_unlock_delalloc(inode, &BTRFS_I(inode)->io_tree,
> - start, end, NULL,
> - EXTENT_CLEAR_UNLOCK_PAGE |
> - EXTENT_CLEAR_DIRTY |
> - EXTENT_CLEAR_DELALLOC |
> - EXTENT_SET_WRITEBACK |
> - EXTENT_END_WRITEBACK);
> + extent_clear_unlock_delalloc(inode, start, end, NULL,
> + EXTENT_DIRTY | EXTENT_DELALLOC,
> + PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
> + PAGE_SET_WRITEBACK | PAGE_END_WRITEBACK);
> if (!trans || IS_ERR(trans))
> btrfs_error(root->fs_info, ret, "Failed to join transaction");
> else
> @@ -770,16 +767,12 @@ retry:
> /*
> * clear dirty, set writeback and unlock the pages.
> */
> - extent_clear_unlock_delalloc(inode,
> - &BTRFS_I(inode)->io_tree,
> - async_extent->start,
> + extent_clear_unlock_delalloc(inode, async_extent->start,
> async_extent->start +
> async_extent->ram_size - 1,
> - NULL, EXTENT_CLEAR_UNLOCK_PAGE |
> - EXTENT_CLEAR_UNLOCK |
> - EXTENT_CLEAR_DELALLOC |
> - EXTENT_CLEAR_DIRTY | EXTENT_SET_WRITEBACK);
> -
> + NULL, EXTENT_LOCKED | EXTENT_DELALLOC |
> + EXTENT_DIRTY, PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
> + PAGE_SET_WRITEBACK);
> ret = btrfs_submit_compressed_write(inode,
> async_extent->start,
> async_extent->ram_size,
> @@ -798,16 +791,13 @@ out:
> out_free_reserve:
> btrfs_free_reserved_extent(root, ins.objectid, ins.offset);
> out_free:
> - extent_clear_unlock_delalloc(inode, &BTRFS_I(inode)->io_tree,
> - async_extent->start,
> + extent_clear_unlock_delalloc(inode, async_extent->start,
> async_extent->start +
> async_extent->ram_size - 1,
> - NULL, EXTENT_CLEAR_UNLOCK_PAGE |
> - EXTENT_CLEAR_UNLOCK |
> - EXTENT_CLEAR_DELALLOC |
> - EXTENT_CLEAR_DIRTY |
> - EXTENT_SET_WRITEBACK |
> - EXTENT_END_WRITEBACK);
> + NULL, EXTENT_LOCKED | EXTENT_DELALLOC |
> + EXTENT_DIRTY, PAGE_UNLOCK |
> + PAGE_CLEAR_DIRTY | PAGE_SET_WRITEBACK |
> + PAGE_END_WRITEBACK);
> kfree(async_extent);
> goto again;
> }
> @@ -892,15 +882,11 @@ static noinline int __cow_file_range(struct btrfs_trans_handle *trans,
> ret = cow_file_range_inline(trans, root, inode,
> start, end, 0, 0, NULL);
> if (ret == 0) {
> - extent_clear_unlock_delalloc(inode,
> - &BTRFS_I(inode)->io_tree,
> - start, end, NULL,
> - EXTENT_CLEAR_UNLOCK_PAGE |
> - EXTENT_CLEAR_UNLOCK |
> - EXTENT_CLEAR_DELALLOC |
> - EXTENT_CLEAR_DIRTY |
> - EXTENT_SET_WRITEBACK |
> - EXTENT_END_WRITEBACK);
> + extent_clear_unlock_delalloc(inode, start, end, NULL,
> + EXTENT_LOCKED | EXTENT_DELALLOC |
> + EXTENT_DIRTY, PAGE_UNLOCK |
> + PAGE_CLEAR_DIRTY | PAGE_SET_WRITEBACK |
> + PAGE_END_WRITEBACK);
>
> *nr_written = *nr_written +
> (end - start + PAGE_CACHE_SIZE) / PAGE_CACHE_SIZE;
> @@ -990,13 +976,13 @@ static noinline int __cow_file_range(struct btrfs_trans_handle *trans,
> * Do set the Private2 bit so we know this page was properly
> * setup for writepage
> */
> - op = unlock ? EXTENT_CLEAR_UNLOCK_PAGE : 0;
> - op |= EXTENT_CLEAR_UNLOCK | EXTENT_CLEAR_DELALLOC |
> - EXTENT_SET_PRIVATE2;
> + op = unlock ? PAGE_UNLOCK : 0;
> + op |= PAGE_SET_PRIVATE2;
>
> - extent_clear_unlock_delalloc(inode, &BTRFS_I(inode)->io_tree,
> - start, start + ram_size - 1,
> - locked_page, op);
> + extent_clear_unlock_delalloc(inode, start,
> + start + ram_size - 1, locked_page,
> + EXTENT_LOCKED | EXTENT_DELALLOC,
> + op);
> disk_num_bytes -= cur_alloc_size;
> num_bytes -= cur_alloc_size;
> alloc_hint = ins.objectid + ins.offset;
> @@ -1008,16 +994,11 @@ out:
> out_reserve:
> btrfs_free_reserved_extent(root, ins.objectid, ins.offset);
> out_unlock:
> - extent_clear_unlock_delalloc(inode,
> - &BTRFS_I(inode)->io_tree,
> - start, end, locked_page,
> - EXTENT_CLEAR_UNLOCK_PAGE |
> - EXTENT_CLEAR_UNLOCK |
> - EXTENT_CLEAR_DELALLOC |
> - EXTENT_CLEAR_DIRTY |
> - EXTENT_SET_WRITEBACK |
> - EXTENT_END_WRITEBACK);
> -
> + extent_clear_unlock_delalloc(inode, start, end, locked_page,
> + EXTENT_LOCKED | EXTENT_DIRTY |
> + EXTENT_DELALLOC, PAGE_UNLOCK |
> + PAGE_CLEAR_DIRTY | PAGE_SET_WRITEBACK |
> + PAGE_END_WRITEBACK);
> goto out;
> }
>
> @@ -1033,15 +1014,12 @@ static noinline int cow_file_range(struct inode *inode,
>
> trans = btrfs_join_transaction(root);
> if (IS_ERR(trans)) {
> - extent_clear_unlock_delalloc(inode,
> - &BTRFS_I(inode)->io_tree,
> - start, end, locked_page,
> - EXTENT_CLEAR_UNLOCK_PAGE |
> - EXTENT_CLEAR_UNLOCK |
> - EXTENT_CLEAR_DELALLOC |
> - EXTENT_CLEAR_DIRTY |
> - EXTENT_SET_WRITEBACK |
> - EXTENT_END_WRITEBACK);
> + extent_clear_unlock_delalloc(inode, start, end, locked_page,
> + EXTENT_LOCKED | EXTENT_DELALLOC |
> + EXTENT_DIRTY, PAGE_UNLOCK |
> + PAGE_CLEAR_DIRTY |
> + PAGE_SET_WRITEBACK |
> + PAGE_END_WRITEBACK);
> return PTR_ERR(trans);
> }
> trans->block_rsv = &root->fs_info->delalloc_block_rsv;
> @@ -1221,15 +1199,12 @@ static noinline int run_delalloc_nocow(struct inode *inode,
>
> path = btrfs_alloc_path();
> if (!path) {
> - extent_clear_unlock_delalloc(inode,
> - &BTRFS_I(inode)->io_tree,
> - start, end, locked_page,
> - EXTENT_CLEAR_UNLOCK_PAGE |
> - EXTENT_CLEAR_UNLOCK |
> - EXTENT_CLEAR_DELALLOC |
> - EXTENT_CLEAR_DIRTY |
> - EXTENT_SET_WRITEBACK |
> - EXTENT_END_WRITEBACK);
> + extent_clear_unlock_delalloc(inode, start, end, locked_page,
> + EXTENT_LOCKED | EXTENT_DELALLOC |
> + EXTENT_DIRTY, PAGE_UNLOCK |
> + PAGE_CLEAR_DIRTY |
> + PAGE_SET_WRITEBACK |
> + PAGE_END_WRITEBACK);
> return -ENOMEM;
> }
>
> @@ -1241,15 +1216,12 @@ static noinline int run_delalloc_nocow(struct inode *inode,
> trans = btrfs_join_transaction(root);
>
> if (IS_ERR(trans)) {
> - extent_clear_unlock_delalloc(inode,
> - &BTRFS_I(inode)->io_tree,
> - start, end, locked_page,
> - EXTENT_CLEAR_UNLOCK_PAGE |
> - EXTENT_CLEAR_UNLOCK |
> - EXTENT_CLEAR_DELALLOC |
> - EXTENT_CLEAR_DIRTY |
> - EXTENT_SET_WRITEBACK |
> - EXTENT_END_WRITEBACK);
> + extent_clear_unlock_delalloc(inode, start, end, locked_page,
> + EXTENT_LOCKED | EXTENT_DELALLOC |
> + EXTENT_DIRTY, PAGE_UNLOCK |
> + PAGE_CLEAR_DIRTY |
> + PAGE_SET_WRITEBACK |
> + PAGE_END_WRITEBACK);
> btrfs_free_path(path);
> return PTR_ERR(trans);
> }
> @@ -1428,11 +1400,11 @@ out_check:
> }
> }
>
> - extent_clear_unlock_delalloc(inode, &BTRFS_I(inode)->io_tree,
> - cur_offset, cur_offset + num_bytes - 1,
> - locked_page, EXTENT_CLEAR_UNLOCK_PAGE |
> - EXTENT_CLEAR_UNLOCK | EXTENT_CLEAR_DELALLOC |
> - EXTENT_SET_PRIVATE2);
> + extent_clear_unlock_delalloc(inode, cur_offset,
> + cur_offset + num_bytes - 1,
> + locked_page, EXTENT_LOCKED |
> + EXTENT_DELALLOC, PAGE_UNLOCK |
> + PAGE_SET_PRIVATE2);
> cur_offset = extent_end;
> if (cur_offset > end)
> break;
> @@ -1460,16 +1432,12 @@ error:
> ret = err;
>
> if (ret && cur_offset < end)
> - extent_clear_unlock_delalloc(inode,
> - &BTRFS_I(inode)->io_tree,
> - cur_offset, end, locked_page,
> - EXTENT_CLEAR_UNLOCK_PAGE |
> - EXTENT_CLEAR_UNLOCK |
> - EXTENT_CLEAR_DELALLOC |
> - EXTENT_CLEAR_DIRTY |
> - EXTENT_SET_WRITEBACK |
> - EXTENT_END_WRITEBACK);
> -
> + extent_clear_unlock_delalloc(inode, cur_offset, end,
> + locked_page, EXTENT_LOCKED |
> + EXTENT_DELALLOC | EXTENT_DIRTY,
> + PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
> + PAGE_SET_WRITEBACK |
> + PAGE_END_WRITEBACK);
> btrfs_free_path(path);
> return ret;
> }
>
prev parent reply other threads:[~2013-07-30 3:59 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-29 15:23 [PATCH] Btrfs: cleanup arguments to extent_clear_unlock_delalloc Josef Bacik
2013-07-30 4:00 ` Miao Xie [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51F73A56.8050105@cn.fujitsu.com \
--to=miaox@cn.fujitsu.com \
--cc=jbacik@fusionio.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).