* [PATCH 0/7] Btrfs: if else cleanups @ 2017-06-07 0:58 Timofey Titovets 2017-06-07 0:58 ` [PATCH 1/7] Btrfs: __compare_inode_defrag decrease max compare count Timofey Titovets ` (6 more replies) 0 siblings, 7 replies; 12+ messages in thread From: Timofey Titovets @ 2017-06-07 0:58 UTC (permalink / raw) To: linux-btrfs; +Cc: Timofey Titovets Most of patches regroup if else logic in attemp to avoid useless checks Last patch convert if else to switch case, because in that place code work with enum and usage of switch case can make more obvious to compiler how to optimize that code This is if else vs case in GCC C++, but i think gcc do the same with C. https://godbolt.org/g/YzPL93 Timofey Titovets (7): Btrfs: __compare_inode_defrag decrease max compare count Btrfs: backref_comp decrease max compare count Btrfs: ref_node_cmp decrease max compare count Btrfs: ref_tree_add remove useless compare Btrfs: add_all_parents skip compare Btrfs: __tree_mod_log_insert decrease max compare count Btrfs: end_workqueue_bio use switch case instead of else if fs/btrfs/backref.c | 36 ++++++++++++++++++++---------------- fs/btrfs/ctree.c | 20 +++++++++++--------- fs/btrfs/disk-io.c | 17 +++++++++++++---- fs/btrfs/file.c | 17 +++++++++-------- fs/btrfs/inode.c | 21 ++++++++++++--------- 5 files changed, 65 insertions(+), 46 deletions(-) -- 2.13.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/7] Btrfs: __compare_inode_defrag decrease max compare count 2017-06-07 0:58 [PATCH 0/7] Btrfs: if else cleanups Timofey Titovets @ 2017-06-07 0:58 ` Timofey Titovets 2017-06-07 0:58 ` [PATCH 2/7] Btrfs: backref_comp " Timofey Titovets ` (5 subsequent siblings) 6 siblings, 0 replies; 12+ messages in thread From: Timofey Titovets @ 2017-06-07 0:58 UTC (permalink / raw) To: linux-btrfs; +Cc: Timofey Titovets In worst case code do 4 comparison, just add some new checks to switch check branch faster now in worst case code do 3 comparison Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com> --- fs/btrfs/file.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index da1096eb1..39efda26c 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -71,16 +71,17 @@ struct inode_defrag { static int __compare_inode_defrag(struct inode_defrag *defrag1, struct inode_defrag *defrag2) { - if (defrag1->root > defrag2->root) - return 1; - else if (defrag1->root < defrag2->root) + if (defrag1->root != defrag2->root) { + if (defrag1->root > defrag2->root) + return 1; return -1; - else if (defrag1->ino > defrag2->ino) - return 1; - else if (defrag1->ino < defrag2->ino) + } + if (defrag1->ino != defrag2->ino) { + if (defrag1->ino > defrag2->ino) + return 1; return -1; - else - return 0; + } + return 0; } /* pop a record for an inode into the defrag tree. The lock -- 2.13.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/7] Btrfs: backref_comp decrease max compare count 2017-06-07 0:58 [PATCH 0/7] Btrfs: if else cleanups Timofey Titovets 2017-06-07 0:58 ` [PATCH 1/7] Btrfs: __compare_inode_defrag decrease max compare count Timofey Titovets @ 2017-06-07 0:58 ` Timofey Titovets 2017-06-07 0:58 ` [PATCH 3/7] Btrfs: ref_node_cmp " Timofey Titovets ` (4 subsequent siblings) 6 siblings, 0 replies; 12+ messages in thread From: Timofey Titovets @ 2017-06-07 0:58 UTC (permalink / raw) To: linux-btrfs; +Cc: Timofey Titovets In worst case code do 6 comparison, just add some new checks to switch check branch faster now in worst case code do 4 comparison Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com> --- fs/btrfs/inode.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 17cbe9306..fa024642a 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -2249,20 +2249,23 @@ struct new_sa_defrag_extent { static int backref_comp(struct sa_defrag_extent_backref *b1, struct sa_defrag_extent_backref *b2) { - if (b1->root_id < b2->root_id) - return -1; - else if (b1->root_id > b2->root_id) + if (b1->root_id != b2->root_id) { + if (b1->root_id < b2->root_id) + return -1; return 1; + } - if (b1->inum < b2->inum) - return -1; - else if (b1->inum > b2->inum) + if (b1->inum != b2->inum) { + if (b1->inum < b2->inum) + return -1; return 1; + } - if (b1->file_pos < b2->file_pos) - return -1; - else if (b1->file_pos > b2->file_pos) + if (b1->file_pos != b2->file_pos) { + if (b1->file_pos < b2->file_pos) + return -1; return 1; + } /* * [------------------------------] ===> (a range of space) -- 2.13.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/7] Btrfs: ref_node_cmp decrease max compare count 2017-06-07 0:58 [PATCH 0/7] Btrfs: if else cleanups Timofey Titovets 2017-06-07 0:58 ` [PATCH 1/7] Btrfs: __compare_inode_defrag decrease max compare count Timofey Titovets 2017-06-07 0:58 ` [PATCH 2/7] Btrfs: backref_comp " Timofey Titovets @ 2017-06-07 0:58 ` Timofey Titovets 2017-06-07 0:58 ` [PATCH 4/7] Btrfs: ref_tree_add remove useless compare Timofey Titovets ` (3 subsequent siblings) 6 siblings, 0 replies; 12+ messages in thread From: Timofey Titovets @ 2017-06-07 0:58 UTC (permalink / raw) To: linux-btrfs; +Cc: Timofey Titovets In worst case code do 8 comparison, just add some new checks to switch check branch faster now in worst case code do 5 comparison Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com> --- fs/btrfs/backref.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c index 24865da63..2e4709e0c 100644 --- a/fs/btrfs/backref.c +++ b/fs/btrfs/backref.c @@ -120,25 +120,29 @@ static void ref_root_free(struct ref_root *ref_tree) */ static int ref_node_cmp(struct ref_node *a, struct ref_node *b) { - if (a->root_id < b->root_id) - return -1; - else if (a->root_id > b->root_id) + if (a->root_id != b->root_id) { + if (a->root_id < b->root_id) + return -1; return 1; + } - if (a->object_id < b->object_id) - return -1; - else if (a->object_id > b->object_id) + if (a->object_id != b->object_id) { + if (a->object_id < b->object_id) + return -1; return 1; + } - if (a->offset < b->offset) - return -1; - else if (a->offset > b->offset) + if (a->offset != b->offset) { + if (a->offset < b->offset) + return -1; return 1; + } - if (a->parent < b->parent) - return -1; - else if (a->parent > b->parent) + if (a->parent != b->parent) { + if (a->parent < b->parent) + return -1; return 1; + } return 0; } -- 2.13.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/7] Btrfs: ref_tree_add remove useless compare 2017-06-07 0:58 [PATCH 0/7] Btrfs: if else cleanups Timofey Titovets ` (2 preceding siblings ...) 2017-06-07 0:58 ` [PATCH 3/7] Btrfs: ref_node_cmp " Timofey Titovets @ 2017-06-07 0:58 ` Timofey Titovets 2017-06-07 0:58 ` [PATCH 5/7] Btrfs: add_all_parents skip compare Timofey Titovets ` (2 subsequent siblings) 6 siblings, 0 replies; 12+ messages in thread From: Timofey Titovets @ 2017-06-07 0:58 UTC (permalink / raw) To: linux-btrfs; +Cc: Timofey Titovets As code already know that (node->ref_mod > 0) else if (node->ref_mod <= 0) - useless So just leave 'else' Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com> --- fs/btrfs/backref.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c index 2e4709e0c..897d664a9 100644 --- a/fs/btrfs/backref.c +++ b/fs/btrfs/backref.c @@ -294,7 +294,7 @@ static int ref_tree_add(struct ref_root *ref_tree, u64 root_id, u64 object_id, if (node->ref_mod > 0) ref_tree->unique_refs += origin_count > 0 ? 0 : 1; - else if (node->ref_mod <= 0) + else ref_tree->unique_refs += origin_count > 0 ? -1 : 0; if (!node->ref_mod) -- 2.13.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/7] Btrfs: add_all_parents skip compare 2017-06-07 0:58 [PATCH 0/7] Btrfs: if else cleanups Timofey Titovets ` (3 preceding siblings ...) 2017-06-07 0:58 ` [PATCH 4/7] Btrfs: ref_tree_add remove useless compare Timofey Titovets @ 2017-06-07 0:58 ` Timofey Titovets 2017-06-07 0:58 ` [PATCH 6/7] Btrfs: __tree_mod_log_insert decrease max compare count Timofey Titovets 2017-06-07 0:58 ` [PATCH 7/7] Btrfs: end_workqueue_bio use switch case instead of else if Timofey Titovets 6 siblings, 0 replies; 12+ messages in thread From: Timofey Titovets @ 2017-06-07 0:58 UTC (permalink / raw) To: linux-btrfs; +Cc: Timofey Titovets By comparison logic if ret => 0 -> ret=0; So lets cleanup compare logic Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com> --- fs/btrfs/backref.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c index 897d664a9..f53045891 100644 --- a/fs/btrfs/backref.c +++ b/fs/btrfs/backref.c @@ -592,10 +592,10 @@ static int add_all_parents(struct btrfs_root *root, struct btrfs_path *path, ret = btrfs_next_old_item(root, path, time_seq); } - if (ret > 0) - ret = 0; - else if (ret < 0) + if (ret < 0) free_inode_elem_list(eie); + else + ret = 0; return ret; } -- 2.13.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 6/7] Btrfs: __tree_mod_log_insert decrease max compare count 2017-06-07 0:58 [PATCH 0/7] Btrfs: if else cleanups Timofey Titovets ` (4 preceding siblings ...) 2017-06-07 0:58 ` [PATCH 5/7] Btrfs: add_all_parents skip compare Timofey Titovets @ 2017-06-07 0:58 ` Timofey Titovets 2017-06-07 9:45 ` Filipe Manana 2017-06-07 0:58 ` [PATCH 7/7] Btrfs: end_workqueue_bio use switch case instead of else if Timofey Titovets 6 siblings, 1 reply; 12+ messages in thread From: Timofey Titovets @ 2017-06-07 0:58 UTC (permalink / raw) To: linux-btrfs; +Cc: Timofey Titovets In worst case code do 4 comparison, just add some new checks to switch check branch faster now in worst case code do 3 comparison Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com> --- fs/btrfs/ctree.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index a3a75f1de..318379531 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -460,15 +460,17 @@ __tree_mod_log_insert(struct btrfs_fs_info *fs_info, struct tree_mod_elem *tm) while (*new) { cur = rb_entry(*new, struct tree_mod_elem, node); parent = *new; - if (cur->logical < tm->logical) - new = &((*new)->rb_left); - else if (cur->logical > tm->logical) - new = &((*new)->rb_right); - else if (cur->seq < tm->seq) - new = &((*new)->rb_left); - else if (cur->seq > tm->seq) - new = &((*new)->rb_right); - else + if (cur->logical != tm->logical) { + if (cur->logical < tm->logical) + new = &((*new)->rb_left); + else + new = &((*new)->rb_right); + } else if (cur->seq != tm->seq) { + if (cur->seq < tm->seq) + new = &((*new)->rb_left); + else + new = &((*new)->rb_right); + } else return -EEXIST; } -- 2.13.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 6/7] Btrfs: __tree_mod_log_insert decrease max compare count 2017-06-07 0:58 ` [PATCH 6/7] Btrfs: __tree_mod_log_insert decrease max compare count Timofey Titovets @ 2017-06-07 9:45 ` Filipe Manana 2017-06-07 11:01 ` Timofey Titovets 0 siblings, 1 reply; 12+ messages in thread From: Filipe Manana @ 2017-06-07 9:45 UTC (permalink / raw) To: Timofey Titovets; +Cc: linux-btrfs@vger.kernel.org On Wed, Jun 7, 2017 at 1:58 AM, Timofey Titovets <nefelim4ag@gmail.com> wrote: > In worst case code do 4 comparison, > just add some new checks to switch check branch faster > now in worst case code do 3 comparison Some comments below. > > Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com> > --- > fs/btrfs/ctree.c | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index a3a75f1de..318379531 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -460,15 +460,17 @@ __tree_mod_log_insert(struct btrfs_fs_info *fs_info, struct tree_mod_elem *tm) > while (*new) { > cur = rb_entry(*new, struct tree_mod_elem, node); > parent = *new; > - if (cur->logical < tm->logical) > - new = &((*new)->rb_left); > - else if (cur->logical > tm->logical) > - new = &((*new)->rb_right); > - else if (cur->seq < tm->seq) > - new = &((*new)->rb_left); > - else if (cur->seq > tm->seq) > - new = &((*new)->rb_right); > - else > + if (cur->logical != tm->logical) { > + if (cur->logical < tm->logical) > + new = &((*new)->rb_left); So now when cur->logical is less then tm->logical, we do 2 comparisons instead of 1. Is this case much less common always? If it were it could be a good change. What guarantees that this change will give better performance for common workloads? What guarantees you the example I just gave isn't typical for most/many common workloads? And most importantly, did you actually do some benchmarks that justify this change? Is this a hot code path that would benefit from such changes? (Since it's the tree mod log, seldom used, I really doubt it). Same comments apply to all the other similar patches you have sent. thanks > + else > + new = &((*new)->rb_right); > + } else if (cur->seq != tm->seq) { > + if (cur->seq < tm->seq) > + new = &((*new)->rb_left); > + else > + new = &((*new)->rb_right); > + } else > return -EEXIST; > } > > -- > 2.13.0 > > -- > 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 -- Filipe David Manana, “Whether you think you can, or you think you can't — you're right.” ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 6/7] Btrfs: __tree_mod_log_insert decrease max compare count 2017-06-07 9:45 ` Filipe Manana @ 2017-06-07 11:01 ` Timofey Titovets 2017-06-13 14:24 ` David Sterba 0 siblings, 1 reply; 12+ messages in thread From: Timofey Titovets @ 2017-06-07 11:01 UTC (permalink / raw) To: Filipe Manana; +Cc: linux-btrfs@vger.kernel.org 2017-06-07 12:45 GMT+03:00 Filipe Manana <fdmanana@gmail.com>: > On Wed, Jun 7, 2017 at 1:58 AM, Timofey Titovets <nefelim4ag@gmail.com> wrote: >> In worst case code do 4 comparison, >> just add some new checks to switch check branch faster >> now in worst case code do 3 comparison > > Some comments below. > >> >> Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com> >> --- >> fs/btrfs/ctree.c | 20 +++++++++++--------- >> 1 file changed, 11 insertions(+), 9 deletions(-) >> >> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c >> index a3a75f1de..318379531 100644 >> --- a/fs/btrfs/ctree.c >> +++ b/fs/btrfs/ctree.c >> @@ -460,15 +460,17 @@ __tree_mod_log_insert(struct btrfs_fs_info *fs_info, struct tree_mod_elem *tm) >> while (*new) { >> cur = rb_entry(*new, struct tree_mod_elem, node); >> parent = *new; >> - if (cur->logical < tm->logical) >> - new = &((*new)->rb_left); >> - else if (cur->logical > tm->logical) >> - new = &((*new)->rb_right); >> - else if (cur->seq < tm->seq) >> - new = &((*new)->rb_left); >> - else if (cur->seq > tm->seq) >> - new = &((*new)->rb_right); >> - else >> + if (cur->logical != tm->logical) { >> + if (cur->logical < tm->logical) >> + new = &((*new)->rb_left); > > So now when cur->logical is less then tm->logical, we do 2 comparisons > instead of 1. > Is this case much less common always? If it were it could be a good change. > What guarantees that this change will give better performance for > common workloads? What guarantees you the example I just gave isn't > typical for most/many common workloads? > > And most importantly, did you actually do some benchmarks that justify > this change? Is this a hot code path that would benefit from such > changes? > (Since it's the tree mod log, seldom used, I really doubt it). > > Same comments apply to all the other similar patches you have sent. > > thanks > Nope, i just lack of expirience to do such benchmarks, same for understand all btrfs code, it's difficult to say that path is hot/cold. I just did reread all btrfs code, analize that i understand and trying fix places that looks questionably. In most cases i can explain why this can help (IMHO): 01 - __compare_inode_defrag - we compare inodes, by my expirience in most distros and setups btrfs have 1-2 subvolumes (root_ids), so in most time root_id1 == root_id2 02 - backref_comp - same 03 - ref_node_cmp - same 04 - ref_tree_add - just a cleanup, compiller do the same https://godbolt.org/g/Ww93ws 05 - add_all_parents - just a cleanup, compiller do the same https://godbolt.org/g/srSGeW 07 - end_workqueue_bio - if i understand correctly this code executed on every write, so by convertion to switch case will save several cpu cycles on every write For that function (06 - __tree_mod_log_insert): i do the same as for 01-03 "by inertia" - as i just not know all code, i just assume that as this fuction work with rb tree, values will be equally distributed Thanks -- Have a nice day, Timofey. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 6/7] Btrfs: __tree_mod_log_insert decrease max compare count 2017-06-07 11:01 ` Timofey Titovets @ 2017-06-13 14:24 ` David Sterba 0 siblings, 0 replies; 12+ messages in thread From: David Sterba @ 2017-06-13 14:24 UTC (permalink / raw) To: Timofey Titovets; +Cc: Filipe Manana, linux-btrfs@vger.kernel.org On Wed, Jun 07, 2017 at 02:01:46PM +0300, Timofey Titovets wrote: > 2017-06-07 12:45 GMT+03:00 Filipe Manana <fdmanana@gmail.com>: > > On Wed, Jun 7, 2017 at 1:58 AM, Timofey Titovets <nefelim4ag@gmail.com> wrote: > >> In worst case code do 4 comparison, > >> just add some new checks to switch check branch faster > >> now in worst case code do 3 comparison > > > > Some comments below. > > > >> > >> Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com> > >> --- > >> fs/btrfs/ctree.c | 20 +++++++++++--------- > >> 1 file changed, 11 insertions(+), 9 deletions(-) > >> > >> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > >> index a3a75f1de..318379531 100644 > >> --- a/fs/btrfs/ctree.c > >> +++ b/fs/btrfs/ctree.c > >> @@ -460,15 +460,17 @@ __tree_mod_log_insert(struct btrfs_fs_info *fs_info, struct tree_mod_elem *tm) > >> while (*new) { > >> cur = rb_entry(*new, struct tree_mod_elem, node); > >> parent = *new; > >> - if (cur->logical < tm->logical) > >> - new = &((*new)->rb_left); > >> - else if (cur->logical > tm->logical) > >> - new = &((*new)->rb_right); > >> - else if (cur->seq < tm->seq) > >> - new = &((*new)->rb_left); > >> - else if (cur->seq > tm->seq) > >> - new = &((*new)->rb_right); > >> - else > >> + if (cur->logical != tm->logical) { > >> + if (cur->logical < tm->logical) > >> + new = &((*new)->rb_left); > > > > So now when cur->logical is less then tm->logical, we do 2 comparisons > > instead of 1. > > Is this case much less common always? If it were it could be a good change. > > What guarantees that this change will give better performance for > > common workloads? What guarantees you the example I just gave isn't > > typical for most/many common workloads? > > > > And most importantly, did you actually do some benchmarks that justify > > this change? Is this a hot code path that would benefit from such > > changes? > > (Since it's the tree mod log, seldom used, I really doubt it). > > > > Same comments apply to all the other similar patches you have sent. > > Nope, i just lack of expirience to do such benchmarks, > same for understand all btrfs code, it's difficult to say that path is hot/cold. So if you call the patchset cleanups and then claim some performance-related improvements, you can expect to be called out to provide numbers or an analysis. I've checked the resulting assembly of "Btrfs: backref_comp decrease max compare count" that does similar conversion of the sequence of ifs. Surprise surprise, the assembly does not differ regarding number of branches. This at mininimum can be done before any profiling to see if this makes sense. If the assembly stays the same, the code readability may be preferred, which is one of the points of cleanup patches. In that case I prefer the current code over your change, but it may be just personal preference and because I'm used to reading the "linear ifs" pattern. > I just did reread all btrfs code, analize that i understand and trying > fix places that looks questionably. > > In most cases i can explain why this can help (IMHO): > 01 - __compare_inode_defrag - we compare inodes, by my expirience > in most distros and setups btrfs have 1-2 subvolumes (root_ids), so in > most time root_id1 == root_id2 > 02 - backref_comp - same > 03 - ref_node_cmp - same > 04 - ref_tree_add - just a cleanup, compiller do the same > https://godbolt.org/g/Ww93ws > 05 - add_all_parents - just a cleanup, compiller do the same > https://godbolt.org/g/srSGeW > 07 - end_workqueue_bio - if i understand correctly this code executed > on every write, > so by convertion to switch case will save several cpu cycles on every write > > For that function (06 - __tree_mod_log_insert): > i do the same as for 01-03 "by inertia" - as i just not know all code, > i just assume that as this fuction work with rb tree, values will be > equally distributed >From my point above, I'll not merge patches 1, 2, 3 and 6. Patches 4 and 5 seem ok. Patch 7 does not result in the equivalent code, as reported by the kbuild robot. But converting the ifs to switch is desired in end_workqueue_bio, namely the second branch as it's not clear how are some of the BTRFS_WQ_ENDIO_* values handled. If you still care, please resend 4 and 5. The changelog does not need to repeat the code, more describe that the change simplifies the code to make it more readable. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 7/7] Btrfs: end_workqueue_bio use switch case instead of else if 2017-06-07 0:58 [PATCH 0/7] Btrfs: if else cleanups Timofey Titovets ` (5 preceding siblings ...) 2017-06-07 0:58 ` [PATCH 6/7] Btrfs: __tree_mod_log_insert decrease max compare count Timofey Titovets @ 2017-06-07 0:58 ` Timofey Titovets 2017-06-07 22:48 ` kbuild test robot 6 siblings, 1 reply; 12+ messages in thread From: Timofey Titovets @ 2017-06-07 0:58 UTC (permalink / raw) To: linux-btrfs; +Cc: Timofey Titovets If arg to "switch case" is determined and it's a consecutive numbers (This is enum btrfs_wq_endio_type) Compiler can create jump table to optimize logic Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com> --- fs/btrfs/disk-io.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 8685d6718..72208826d 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -802,18 +802,27 @@ static void end_workqueue_bio(struct bio *bio) end_io_wq->error = bio->bi_error; if (bio_op(bio) == REQ_OP_WRITE) { - if (end_io_wq->metadata == BTRFS_WQ_ENDIO_METADATA) { + switch (end_io_wq->metadata) { + case BTRFS_WQ_ENDIO_DATA: + wq = fs_info->endio_write_workers; + func = btrfs_endio_write_helper; + break; + case BTRFS_WQ_ENDIO_METADATA: wq = fs_info->endio_meta_write_workers; func = btrfs_endio_meta_write_helper; - } else if (end_io_wq->metadata == BTRFS_WQ_ENDIO_FREE_SPACE) { + break; + case BTRFS_WQ_ENDIO_FREE_SPACE: wq = fs_info->endio_freespace_worker; func = btrfs_freespace_write_helper; - } else if (end_io_wq->metadata == BTRFS_WQ_ENDIO_RAID56) { + break; + case BTRFS_WQ_ENDIO_RAID56: wq = fs_info->endio_raid56_workers; func = btrfs_endio_raid56_helper; - } else { + break; + case BTRFS_WQ_ENDIO_DIO_REPAIR: wq = fs_info->endio_write_workers; func = btrfs_endio_write_helper; + break; } } else { if (unlikely(end_io_wq->metadata == -- 2.13.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 7/7] Btrfs: end_workqueue_bio use switch case instead of else if 2017-06-07 0:58 ` [PATCH 7/7] Btrfs: end_workqueue_bio use switch case instead of else if Timofey Titovets @ 2017-06-07 22:48 ` kbuild test robot 0 siblings, 0 replies; 12+ messages in thread From: kbuild test robot @ 2017-06-07 22:48 UTC (permalink / raw) To: Timofey Titovets; +Cc: kbuild-all, linux-btrfs, Timofey Titovets [-- Attachment #1: Type: text/plain, Size: 2371 bytes --] Hi Timofey, [auto build test WARNING on next-20170605] [also build test WARNING on v4.12-rc4] [cannot apply to btrfs/next v4.9-rc8 v4.9-rc7 v4.9-rc6] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Timofey-Titovets/Btrfs-if-else-cleanups/20170608-055223 config: x86_64-randconfig-x016-201723 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 Note: it may well be a FALSE warning. FWIW you are at least aware of it now. http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings All warnings (new ones prefixed by >>): fs//btrfs/disk-io.c: In function 'end_workqueue_bio': >> fs//btrfs/disk-io.c:843:2: warning: 'func' may be used uninitialized in this function [-Wmaybe-uninitialized] btrfs_init_work(&end_io_wq->work, func, end_workqueue_fn, NULL, NULL); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> fs//btrfs/disk-io.c:844:2: warning: 'wq' may be used uninitialized in this function [-Wmaybe-uninitialized] btrfs_queue_work(wq, &end_io_wq->work); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +/func +843 fs//btrfs/disk-io.c 9e0af237 Liu Bo 2014-08-15 837 } else { 9e0af237 Liu Bo 2014-08-15 838 wq = fs_info->endio_workers; 9e0af237 Liu Bo 2014-08-15 839 func = btrfs_endio_helper; d20f7043 Chris Mason 2008-12-08 840 } ce9adaa5 Chris Mason 2008-04-09 841 } ce9adaa5 Chris Mason 2008-04-09 842 9e0af237 Liu Bo 2014-08-15 @843 btrfs_init_work(&end_io_wq->work, func, end_workqueue_fn, NULL, NULL); 9e0af237 Liu Bo 2014-08-15 @844 btrfs_queue_work(wq, &end_io_wq->work); ce9adaa5 Chris Mason 2008-04-09 845 } ce9adaa5 Chris Mason 2008-04-09 846 22c59948 Chris Mason 2008-04-09 847 int btrfs_bio_wq_end_io(struct btrfs_fs_info *info, struct bio *bio, :::::: The code at line 843 was first introduced by commit :::::: 9e0af23764344f7f1b68e4eefbe7dc865018b63d Btrfs: fix task hang under heavy compressed write :::::: TO: Liu Bo <bo.li.liu@oracle.com> :::::: CC: Chris Mason <clm@fb.com> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 24830 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-06-13 14:25 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-07 0:58 [PATCH 0/7] Btrfs: if else cleanups Timofey Titovets 2017-06-07 0:58 ` [PATCH 1/7] Btrfs: __compare_inode_defrag decrease max compare count Timofey Titovets 2017-06-07 0:58 ` [PATCH 2/7] Btrfs: backref_comp " Timofey Titovets 2017-06-07 0:58 ` [PATCH 3/7] Btrfs: ref_node_cmp " Timofey Titovets 2017-06-07 0:58 ` [PATCH 4/7] Btrfs: ref_tree_add remove useless compare Timofey Titovets 2017-06-07 0:58 ` [PATCH 5/7] Btrfs: add_all_parents skip compare Timofey Titovets 2017-06-07 0:58 ` [PATCH 6/7] Btrfs: __tree_mod_log_insert decrease max compare count Timofey Titovets 2017-06-07 9:45 ` Filipe Manana 2017-06-07 11:01 ` Timofey Titovets 2017-06-13 14:24 ` David Sterba 2017-06-07 0:58 ` [PATCH 7/7] Btrfs: end_workqueue_bio use switch case instead of else if Timofey Titovets 2017-06-07 22:48 ` kbuild test robot
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).