* [PATCH 1/4 linux-next] xfs: use swap() in xfs_dir2_leafn_rebalance()
@ 2015-05-18 17:13 Fabian Frederick
2015-05-18 17:13 ` [PATCH 2/4 linux-next] xfs: use swap() in xfs_lock_two_inodes() Fabian Frederick
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Fabian Frederick @ 2015-05-18 17:13 UTC (permalink / raw)
To: linux-kernel; +Cc: Fabian Frederick, Dave Chinner, xfs
Use kernel.h macro definition.
Also remove assignment in if condition (checkpatch error)
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
fs/xfs/libxfs/xfs_dir2_node.c | 8 ++------
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
fs/xfs/libxfs/xfs_dir2_node.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index 41b80d3..84442a6 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -967,13 +967,10 @@ xfs_dir2_leafn_rebalance(
/*
* If the block order is wrong, swap the arguments.
*/
- if ((swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp))) {
- xfs_da_state_blk_t *tmp; /* temp for block swap */
+ swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp);
+ if (swap)
+ swap(blk1, blk2);
- tmp = blk1;
- blk1 = blk2;
- blk2 = tmp;
- }
leaf1 = blk1->bp->b_addr;
leaf2 = blk2->bp->b_addr;
dp->d_ops->leaf_hdr_from_disk(&hdr1, leaf1);
--
2.4.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4 linux-next] xfs: use swap() in xfs_lock_two_inodes()
2015-05-18 17:13 [PATCH 1/4 linux-next] xfs: use swap() in xfs_dir2_leafn_rebalance() Fabian Frederick
@ 2015-05-18 17:13 ` Fabian Frederick
2015-05-18 17:13 ` [PATCH 3/4 linux-next] xfs: use swap() in xfs_attr3_leaf_rebalance() Fabian Frederick
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Fabian Frederick @ 2015-05-18 17:13 UTC (permalink / raw)
To: linux-kernel; +Cc: Fabian Frederick, Dave Chinner, xfs
Use kernel.h macro definition.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
fs/xfs/xfs_inode.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index d6ebc85..d4f3a09 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -500,7 +500,6 @@ xfs_lock_two_inodes(
xfs_inode_t *ip1,
uint lock_mode)
{
- xfs_inode_t *temp;
int attempts = 0;
xfs_log_item_t *lp;
@@ -512,11 +511,8 @@ xfs_lock_two_inodes(
ASSERT(ip0->i_ino != ip1->i_ino);
- if (ip0->i_ino > ip1->i_ino) {
- temp = ip0;
- ip0 = ip1;
- ip1 = temp;
- }
+ if (ip0->i_ino > ip1->i_ino)
+ swap(ip0, ip1);
again:
xfs_ilock(ip0, xfs_lock_inumorder(lock_mode, 0));
--
2.4.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/4 linux-next] xfs: use swap() in xfs_attr3_leaf_rebalance()
2015-05-18 17:13 [PATCH 1/4 linux-next] xfs: use swap() in xfs_dir2_leafn_rebalance() Fabian Frederick
2015-05-18 17:13 ` [PATCH 2/4 linux-next] xfs: use swap() in xfs_lock_two_inodes() Fabian Frederick
@ 2015-05-18 17:13 ` Fabian Frederick
2015-05-18 17:13 ` [PATCH 4/4 linux-next] xfs: use swap() in xfs_da3_node_rebalance() Fabian Frederick
2015-05-18 17:19 ` [PATCH 1/4 linux-next] xfs: use swap() in xfs_dir2_leafn_rebalance() Al Viro
3 siblings, 0 replies; 7+ messages in thread
From: Fabian Frederick @ 2015-05-18 17:13 UTC (permalink / raw)
To: linux-kernel; +Cc: Fabian Frederick, Dave Chinner, xfs
Use kernel.h macro definition.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
fs/xfs/libxfs/xfs_attr_leaf.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 04e79d5..a346455 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -1481,17 +1481,10 @@ xfs_attr3_leaf_rebalance(
*/
swap = 0;
if (xfs_attr3_leaf_order(blk1->bp, &ichdr1, blk2->bp, &ichdr2)) {
- struct xfs_da_state_blk *tmp_blk;
- struct xfs_attr3_icleaf_hdr tmp_ichdr;
-
- tmp_blk = blk1;
- blk1 = blk2;
- blk2 = tmp_blk;
+ swap(blk1, blk2);
/* struct copies to swap them rather than reconverting */
- tmp_ichdr = ichdr1;
- ichdr1 = ichdr2;
- ichdr2 = tmp_ichdr;
+ swap(ichdr1, ichdr2);
leaf1 = blk1->bp->b_addr;
leaf2 = blk2->bp->b_addr;
--
2.4.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/4 linux-next] xfs: use swap() in xfs_da3_node_rebalance()
2015-05-18 17:13 [PATCH 1/4 linux-next] xfs: use swap() in xfs_dir2_leafn_rebalance() Fabian Frederick
2015-05-18 17:13 ` [PATCH 2/4 linux-next] xfs: use swap() in xfs_lock_two_inodes() Fabian Frederick
2015-05-18 17:13 ` [PATCH 3/4 linux-next] xfs: use swap() in xfs_attr3_leaf_rebalance() Fabian Frederick
@ 2015-05-18 17:13 ` Fabian Frederick
2015-05-18 17:19 ` [PATCH 1/4 linux-next] xfs: use swap() in xfs_dir2_leafn_rebalance() Al Viro
3 siblings, 0 replies; 7+ messages in thread
From: Fabian Frederick @ 2015-05-18 17:13 UTC (permalink / raw)
To: linux-kernel; +Cc: Fabian Frederick, Dave Chinner, xfs
Use kernel.h macro definition.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
fs/xfs/libxfs/xfs_da_btree.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 2385f8c..f0101a8 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -734,7 +734,6 @@ xfs_da3_node_rebalance(
{
struct xfs_da_intnode *node1;
struct xfs_da_intnode *node2;
- struct xfs_da_intnode *tmpnode;
struct xfs_da_node_entry *btree1;
struct xfs_da_node_entry *btree2;
struct xfs_da_node_entry *btree_s;
@@ -764,9 +763,7 @@ xfs_da3_node_rebalance(
((be32_to_cpu(btree2[0].hashval) < be32_to_cpu(btree1[0].hashval)) ||
(be32_to_cpu(btree2[nodehdr2.count - 1].hashval) <
be32_to_cpu(btree1[nodehdr1.count - 1].hashval)))) {
- tmpnode = node1;
- node1 = node2;
- node2 = tmpnode;
+ swap(node1, node2);
dp->d_ops->node_hdr_from_disk(&nodehdr1, node1);
dp->d_ops->node_hdr_from_disk(&nodehdr2, node2);
btree1 = dp->d_ops->node_tree_p(node1);
--
2.4.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4 linux-next] xfs: use swap() in xfs_dir2_leafn_rebalance()
2015-05-18 17:13 [PATCH 1/4 linux-next] xfs: use swap() in xfs_dir2_leafn_rebalance() Fabian Frederick
` (2 preceding siblings ...)
2015-05-18 17:13 ` [PATCH 4/4 linux-next] xfs: use swap() in xfs_da3_node_rebalance() Fabian Frederick
@ 2015-05-18 17:19 ` Al Viro
2015-05-18 18:06 ` Fabian Frederick
3 siblings, 1 reply; 7+ messages in thread
From: Al Viro @ 2015-05-18 17:19 UTC (permalink / raw)
To: Fabian Frederick; +Cc: linux-kernel, Dave Chinner, xfs
On Mon, May 18, 2015 at 07:13:48PM +0200, Fabian Frederick wrote:
> * If the block order is wrong, swap the arguments.
> */
> - if ((swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp))) {
> - xfs_da_state_blk_t *tmp; /* temp for block swap */
> + swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp);
> + if (swap)
> + swap(blk1, blk2);
Egads... Have you even read what you'd written? Yes, sure, preprocessor
will do the right thing, but it's a very noticable annoyance for somebody
reading that. Rename the bleeding flag, please.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4 linux-next] xfs: use swap() in xfs_dir2_leafn_rebalance()
2015-05-18 17:19 ` [PATCH 1/4 linux-next] xfs: use swap() in xfs_dir2_leafn_rebalance() Al Viro
@ 2015-05-18 18:06 ` Fabian Frederick
2015-05-18 21:37 ` Dave Chinner
0 siblings, 1 reply; 7+ messages in thread
From: Fabian Frederick @ 2015-05-18 18:06 UTC (permalink / raw)
To: Al Viro; +Cc: linux-kernel, Dave Chinner, xfs
> On 18 May 2015 at 19:19 Al Viro <viro@ZenIV.linux.org.uk> wrote:
>
>
> On Mon, May 18, 2015 at 07:13:48PM +0200, Fabian Frederick wrote:
> > * If the block order is wrong, swap the arguments.
> > */
> > - if ((swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp))) {
> > - xfs_da_state_blk_t *tmp; /* temp for block swap */
> > + swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp);
> > + if (swap)
> > + swap(blk1, blk2);
>
> Egads... Have you even read what you'd written? Yes, sure, preprocessor
> will do the right thing, but it's a very noticable annoyance for somebody
> reading that. Rename the bleeding flag, please.
I wanted to focus on the swap() update in this small patchset (some other things
should be done in there like have xfs_dir2_leafn_order() return bool) but I can
rename it in something like need_swap. Do I need to resend the 4 patches Dave ?
Regards,
Fabian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4 linux-next] xfs: use swap() in xfs_dir2_leafn_rebalance()
2015-05-18 18:06 ` Fabian Frederick
@ 2015-05-18 21:37 ` Dave Chinner
0 siblings, 0 replies; 7+ messages in thread
From: Dave Chinner @ 2015-05-18 21:37 UTC (permalink / raw)
To: Fabian Frederick; +Cc: Al Viro, linux-kernel, xfs
On Mon, May 18, 2015 at 08:06:13PM +0200, Fabian Frederick wrote:
>
>
> > On 18 May 2015 at 19:19 Al Viro <viro@ZenIV.linux.org.uk> wrote:
> >
> >
> > On Mon, May 18, 2015 at 07:13:48PM +0200, Fabian Frederick wrote:
> > > * If the block order is wrong, swap the arguments.
> > > */
> > > - if ((swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp))) {
> > > - xfs_da_state_blk_t *tmp; /* temp for block swap */
> > > + swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp);
> > > + if (swap)
> > > + swap(blk1, blk2);
> >
> > Egads... Have you even read what you'd written? Yes, sure, preprocessor
> > will do the right thing, but it's a very noticable annoyance for somebody
> > reading that. Rename the bleeding flag, please.
>
> I wanted to focus on the swap() update in this small patchset (some other things
> should be done in there like have xfs_dir2_leafn_order() return bool) but I can
> rename it in something like need_swap. Do I need to resend the 4 patches Dave ?
4 patches is 3 patches too many for noise like this. Anyway, two of
the patches have the same local "swap" variable problem; the context
is "swap order" not "need swap".
FWIW, I am not a fan of changing the code for no actual gain - the
compiled code is identical, there are no stack savings, and now I
have to look at an extra file to work out what the code does. If
you're changing the code and this is prep work for a large series,
then by all means clean the code up. But otherwise, changes like
this just mean work that other developers have in progress need
rebasing....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-05-18 21:37 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-18 17:13 [PATCH 1/4 linux-next] xfs: use swap() in xfs_dir2_leafn_rebalance() Fabian Frederick
2015-05-18 17:13 ` [PATCH 2/4 linux-next] xfs: use swap() in xfs_lock_two_inodes() Fabian Frederick
2015-05-18 17:13 ` [PATCH 3/4 linux-next] xfs: use swap() in xfs_attr3_leaf_rebalance() Fabian Frederick
2015-05-18 17:13 ` [PATCH 4/4 linux-next] xfs: use swap() in xfs_da3_node_rebalance() Fabian Frederick
2015-05-18 17:19 ` [PATCH 1/4 linux-next] xfs: use swap() in xfs_dir2_leafn_rebalance() Al Viro
2015-05-18 18:06 ` Fabian Frederick
2015-05-18 21:37 ` Dave Chinner
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).