From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5C8AFC433F5 for ; Wed, 9 Mar 2022 11:45:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231390AbiCILqs (ORCPT ); Wed, 9 Mar 2022 06:46:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:32990 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229498AbiCILqr (ORCPT ); Wed, 9 Mar 2022 06:46:47 -0500 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E9A501712B7 for ; Wed, 9 Mar 2022 03:45:48 -0800 (PST) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 934331F381; Wed, 9 Mar 2022 11:45:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1646826347; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=BcCjc6cPSsnPlitvrffNBVdapji1mH3oK0/hUQEnKw4=; b=eDgCe/9l10Yf9g0dfQH1ceMPJXSoVWdCEijTRU/NVbs2cAv0DEnsS1uTlk30ZXdGcq4brm ItQO/ZgI4O53V4LyV8lXEjTCABvT2KXOabYeNrzmw5oWiPOMurVxIsW2zWV14xBoTDTIoP qHXma2r9PawwdazjUwRGRsAJiWKCbC0= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 691D813D7A; Wed, 9 Mar 2022 11:45:47 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 21C1FmuTKGIXbQAAMHmgww (envelope-from ); Wed, 09 Mar 2022 11:45:47 +0000 Message-ID: <300c4868-46e3-9862-3355-cd0091f2ae3e@suse.com> Date: Wed, 9 Mar 2022 13:45:46 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH 06/13] btrfs-progs: btrfs_item_size_nr/btrfs_item_offset_nr everywhere Content-Language: en-US To: Josef Bacik , linux-btrfs@vger.kernel.org, kernel-team@fb.com References: From: Nikolay Borisov In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org On 23.02.22 г. 0:26 ч., Josef Bacik wrote: > We have this pattern in a lot of places > > item = btrfs_item_nr(slot); > btrfs_item_size(leaf, item); > btrfs_item_offset(leaf, item); > > when we could simply use > > btrfs_item_size_nr(leaf, slot); > btrfs_item_offset_nr(leaf, slot); > > Fix all callers of btrfs_item_size() and btrfs_item_offset() to use the > _nr variation of the helpers. > > Signed-off-by: Josef Bacik > --- > check/main.c | 8 ++++---- > image/main.c | 2 +- > kernel-shared/backref.c | 4 +--- > kernel-shared/ctree.c | 24 ++++++++++++------------ > kernel-shared/dir-item.c | 6 ++---- > kernel-shared/inode-item.c | 4 +--- > kernel-shared/print-tree.c | 6 +++--- > 7 files changed, 24 insertions(+), 30 deletions(-) > > diff --git a/check/main.c b/check/main.c > index e9ac94cc..76eb8d54 100644 > --- a/check/main.c > +++ b/check/main.c > @@ -4222,10 +4222,10 @@ static int swap_values(struct btrfs_root *root, struct btrfs_path *path, > item2 = btrfs_item_nr(slot + 1); > btrfs_item_key_to_cpu(buf, &k1, slot); > btrfs_item_key_to_cpu(buf, &k2, slot + 1); > - item1_offset = btrfs_item_offset(buf, item1); > - item2_offset = btrfs_item_offset(buf, item2); > - item1_size = btrfs_item_size(buf, item1); > - item2_size = btrfs_item_size(buf, item2); > + item1_offset = btrfs_item_offset_nr(buf, slot); > + item2_offset = btrfs_item_offset_nr(buf, slot + 1); > + item1_size = btrfs_item_size_nr(buf, slot); > + item2_size = btrfs_item_size_nr(buf, slot + 1); > > item1_data = malloc(item1_size); > if (!item1_data) > diff --git a/image/main.c b/image/main.c > index 3125163d..e953d981 100644 > --- a/image/main.c > +++ b/image/main.c > @@ -1239,7 +1239,7 @@ static void truncate_item(struct extent_buffer *eb, int slot, u32 new_size) > for (i = slot; i < nritems; i++) { > u32 ioff; > item = btrfs_item_nr(i); > - ioff = btrfs_item_offset(eb, item); > + ioff = btrfs_item_offset_nr(eb, i); > btrfs_set_item_offset(eb, item, ioff + size_diff); > } > > diff --git a/kernel-shared/backref.c b/kernel-shared/backref.c > index f1a638ed..327599b7 100644 > --- a/kernel-shared/backref.c > +++ b/kernel-shared/backref.c > @@ -1417,7 +1417,6 @@ static int iterate_inode_refs(u64 inum, struct btrfs_root *fs_root, > u64 parent = 0; > int found = 0; > struct extent_buffer *eb; > - struct btrfs_item *item; > struct btrfs_inode_ref *iref; > struct btrfs_key found_key; > > @@ -1442,10 +1441,9 @@ static int iterate_inode_refs(u64 inum, struct btrfs_root *fs_root, > extent_buffer_get(eb); > btrfs_release_path(path); > > - item = btrfs_item_nr(slot); > iref = btrfs_item_ptr(eb, slot, struct btrfs_inode_ref); > > - for (cur = 0; cur < btrfs_item_size(eb, item); cur += len) { > + for (cur = 0; cur < btrfs_item_size_nr(eb, slot); cur += len) { > name_len = btrfs_inode_ref_name_len(eb, iref); > /* path must be released before calling iterate()! */ > pr_debug("following ref at offset %u for inode %llu in " > diff --git a/kernel-shared/ctree.c b/kernel-shared/ctree.c > index 10b22b2c..fc661aeb 100644 > --- a/kernel-shared/ctree.c > +++ b/kernel-shared/ctree.c > @@ -2041,7 +2041,7 @@ static int push_leaf_right(struct btrfs_trans_handle *trans, struct btrfs_root > if (path->slots[0] == i) > push_space += data_size + sizeof(*item); item is no longer used in this while so the assignment done above this if can also be removed. Heck, I'd even go as far as moving the variable declaration inside the for() loop further down and convert all the sizeof(*item) calls to either sizeof(struct btrfs_item) or make a const size_t item_size = sizeof(struct btrfs_item) atop the function and use that. > > - this_item_size = btrfs_item_size(left, item); > + this_item_size = btrfs_item_size_nr(left, i); > if (this_item_size + sizeof(*item) + push_space > free_space) > break; > push_items++; > @@ -2092,7 +2092,7 @@ static int push_leaf_right(struct btrfs_trans_handle *trans, struct btrfs_root > push_space = BTRFS_LEAF_DATA_SIZE(root->fs_info); > for (i = 0; i < right_nritems; i++) { > item = btrfs_item_nr(i); Then this line could be turned into: struct btrfs_item item = btrfs_item_nr(i); > - push_space -= btrfs_item_size(right, item); > + push_space -= btrfs_item_size_nr(right, i); > btrfs_set_item_offset(right, item, push_space); And item would be confined to the single site here. > } > > @@ -2187,7 +2187,7 @@ static int push_leaf_left(struct btrfs_trans_handle *trans, struct btrfs_root > if (path->slots[0] == i) > push_space += data_size + sizeof(*item); > > - this_item_size = btrfs_item_size(right, item); > + this_item_size = btrfs_item_size_nr(right, i); > if (this_item_size + sizeof(*item) + push_space > free_space) > break; > > @@ -2224,7 +2224,7 @@ static int push_leaf_left(struct btrfs_trans_handle *trans, struct btrfs_root > u32 ioff; > > item = btrfs_item_nr(i); > - ioff = btrfs_item_offset(left, item); > + ioff = btrfs_item_offset_nr(left, i); > btrfs_set_item_offset(left, item, > ioff - (BTRFS_LEAF_DATA_SIZE(root->fs_info) - > old_left_item_size)); > @@ -2256,7 +2256,7 @@ static int push_leaf_left(struct btrfs_trans_handle *trans, struct btrfs_root > push_space = BTRFS_LEAF_DATA_SIZE(root->fs_info); > for (i = 0; i < right_nritems; i++) { > item = btrfs_item_nr(i); > - push_space = push_space - btrfs_item_size(right, item); > + push_space = push_space - btrfs_item_size_nr(right, i); > btrfs_set_item_offset(right, item, push_space); > } nit: Same strategy can be applied to push_leaf_left as the one in pusth_leaf_right > > > @@ -3029,7 +3029,7 @@ int btrfs_del_items(struct btrfs_trans_handle *trans, struct btrfs_root *root, > u32 ioff; > > item = btrfs_item_nr(i); nit: btrfs_item is used only in this loop so why not move it to reduce the variable's scope. > - ioff = btrfs_item_offset(leaf, item); > + ioff = btrfs_item_offset_nr(leaf, i); > btrfs_set_item_offset(leaf, item, ioff + dsize); > } >