From: Eric Sandeen <sandeen@redhat.com>
To: Ross Kirk <ross.kirk@gmail.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] btrfs: remove unused parameter from btrfs_header_fsid
Date: Fri, 20 Sep 2013 09:51:46 -0500 [thread overview]
Message-ID: <523C6102.3030907@redhat.com> (raw)
In-Reply-To: <1379683626-29724-1-git-send-email-ross.kirk@gmail.com>
On 9/20/13 8:27 AM, Ross Kirk wrote:
> Remove unused parameter, 'eb'. Unused since introduction in
> 5f39d397dfbe140a14edecd4e73c34ce23c4f9ee
>
> Signed-off-by: Ross Kirk <ross.kirk@gmail.com>
A few points; was this against latest upstream?
Upstream, fba6aa75654394fccf2530041e9451414c28084f removed
all the superfluous (unsigned long) casts when this function
gets called. So I think you need to rebase this.
(that commit would be great to apply to userspace as well to keep things
more in sync).
But it seems like this could be cleaned up even more:
static inline unsigned long btrfs_header_fsid(struct extent_buffer *eb)
{
return offsetof(struct btrfs_header, fsid);
}
really? We have a function which just returns a static value from offsetof?
And to a caveman-code-reader like me, I'd sure expect "btrfs_header_fsid" to
give me an fsid, not an offset of a structure member. :(
And as for types, I'm not sure why write_extent_buffer takes an unsigned long
for start; if nothing else it seems to actually be used as a size_t inside the
function. Argh.
Well -
I suppose for now, removing the unused var is a decent start, after you rebase.
btrfs_header_chunk_tree_uuid() could get similar treatments in kernelspace
and/or userspace. As could btrfs_leaf_data().
Welcome to "peeling the onion." :)
-Eric
> ---
> fs/btrfs/ctree.c | 10 +++++-----
> fs/btrfs/ctree.h | 2 +-
> fs/btrfs/disk-io.c | 6 +++---
> fs/btrfs/ioctl.c | 2 +-
> 4 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 63cde75..febc672 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -275,7 +275,7 @@ int btrfs_copy_root(struct btrfs_trans_handle *trans,
> btrfs_set_header_owner(cow, new_root_objectid);
>
> write_extent_buffer(cow, root->fs_info->fsid,
> - (unsigned long)btrfs_header_fsid(cow),
> + (unsigned long)btrfs_header_fsid(),
> BTRFS_FSID_SIZE);
>
> WARN_ON(btrfs_header_generation(buf) > trans->transid);
> @@ -1047,7 +1047,7 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans,
> btrfs_set_header_owner(cow, root->root_key.objectid);
>
> write_extent_buffer(cow, root->fs_info->fsid,
> - (unsigned long)btrfs_header_fsid(cow),
> + (unsigned long)btrfs_header_fsid(),
> BTRFS_FSID_SIZE);
>
> ret = update_ref_for_cow(trans, root, buf, cow, &last_ref);
> @@ -3145,7 +3145,7 @@ static noinline int insert_new_root(struct btrfs_trans_handle *trans,
> btrfs_set_header_owner(c, root->root_key.objectid);
>
> write_extent_buffer(c, root->fs_info->fsid,
> - (unsigned long)btrfs_header_fsid(c),
> + (unsigned long)btrfs_header_fsid(),
> BTRFS_FSID_SIZE);
>
> write_extent_buffer(c, root->fs_info->chunk_tree_uuid,
> @@ -3285,7 +3285,7 @@ static noinline int split_node(struct btrfs_trans_handle *trans,
> btrfs_set_header_backref_rev(split, BTRFS_MIXED_BACKREF_REV);
> btrfs_set_header_owner(split, root->root_key.objectid);
> write_extent_buffer(split, root->fs_info->fsid,
> - (unsigned long)btrfs_header_fsid(split),
> + (unsigned long)btrfs_header_fsid(),
> BTRFS_FSID_SIZE);
> write_extent_buffer(split, root->fs_info->chunk_tree_uuid,
> (unsigned long)btrfs_header_chunk_tree_uuid(split),
> @@ -4041,7 +4041,7 @@ again:
> btrfs_set_header_owner(right, root->root_key.objectid);
> btrfs_set_header_level(right, 0);
> write_extent_buffer(right, root->fs_info->fsid,
> - (unsigned long)btrfs_header_fsid(right),
> + (unsigned long)btrfs_header_fsid(),
> BTRFS_FSID_SIZE);
>
> write_extent_buffer(right, root->fs_info->chunk_tree_uuid,
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index e0bab4d..972a2f8 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2602,7 +2602,7 @@ static inline void btrfs_set_header_backref_rev(struct extent_buffer *eb,
> btrfs_set_header_flags(eb, flags);
> }
>
> -static inline u8 *btrfs_header_fsid(struct extent_buffer *eb)
> +static inline u8 *btrfs_header_fsid(void)
> {
> unsigned long ptr = offsetof(struct btrfs_header, fsid);
> return (u8 *)ptr;
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 3c2886c..e038693 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -497,7 +497,7 @@ static int check_tree_block_fsid(struct btrfs_root *root,
> u8 fsid[BTRFS_UUID_SIZE];
> int ret = 1;
>
> - read_extent_buffer(eb, fsid, (unsigned long)btrfs_header_fsid(eb),
> + read_extent_buffer(eb, fsid, (unsigned long)btrfs_header_fsid(),
> BTRFS_FSID_SIZE);
> while (fs_devices) {
> if (!memcmp(fsid, fs_devices->fsid, BTRFS_FSID_SIZE)) {
> @@ -1291,7 +1291,7 @@ struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
> root->node = leaf;
>
> write_extent_buffer(leaf, fs_info->fsid,
> - (unsigned long)btrfs_header_fsid(leaf),
> + (unsigned long)btrfs_header_fsid(),
> BTRFS_FSID_SIZE);
> write_extent_buffer(leaf, fs_info->chunk_tree_uuid,
> (unsigned long)btrfs_header_chunk_tree_uuid(leaf),
> @@ -1378,7 +1378,7 @@ static struct btrfs_root *alloc_log_tree(struct btrfs_trans_handle *trans,
> root->node = leaf;
>
> write_extent_buffer(root->node, root->fs_info->fsid,
> - (unsigned long)btrfs_header_fsid(root->node),
> + (unsigned long)btrfs_header_fsid(),
> BTRFS_FSID_SIZE);
> btrfs_mark_buffer_dirty(root->node);
> btrfs_tree_unlock(root->node);
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 0e17a30..464e569 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -426,7 +426,7 @@ static noinline int create_subvol(struct inode *dir,
> btrfs_set_header_owner(leaf, objectid);
>
> write_extent_buffer(leaf, root->fs_info->fsid,
> - (unsigned long)btrfs_header_fsid(leaf),
> + (unsigned long)btrfs_header_fsid(),
> BTRFS_FSID_SIZE);
> write_extent_buffer(leaf, root->fs_info->chunk_tree_uuid,
> (unsigned long)btrfs_header_chunk_tree_uuid(leaf),
>
next prev parent reply other threads:[~2013-09-20 14:51 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-20 13:27 [PATCH] btrfs: remove unused parameter from btrfs_header_fsid Ross Kirk
2013-09-20 14:51 ` Eric Sandeen [this message]
2013-09-24 9:07 ` [PATCH v2] " Ross Kirk
2013-09-24 9:12 ` [PATCH v3] " Ross Kirk
2013-09-24 15:01 ` Eric Sandeen
2013-09-24 14:33 ` [PATCH v2] " Eric Sandeen
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=523C6102.3030907@redhat.com \
--to=sandeen@redhat.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=ross.kirk@gmail.com \
/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).