Skip to contentSkip to conversation

ext4: use swap macro in mext_page_double_lock

PATCH Superseded Gustavo A. R. Silva 2 participants 3 messages Jul 9, 2018 – Jul 22, 2018 ext4 file system

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ext4: use swap macro in mext_page_double_lock

Make use of the swap macro and remove unnecessary variable *tmp*. This makes the code easier to read and maintain. Also, reduces the stack usage.

This code was detected with the help of Coccinelle.

---
 fs/ext4/move_extent.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
fs/ext4/move_extent.c +1 -3
index 8e17efd..a409ff7 100644
@@ -134,9 +134,7 @@ mext_page_double_lock(struct inode *inode1, struct inode *inode2,
 		mapping[0] = inode1->i_mapping;
 		mapping[1] = inode2->i_mapping;
 	} else {
-		pgoff_t tmp = index1;
-		index1 = index2;
-		index2 = tmp;
+		swap(index1, index2);
 		mapping[0] = inode2->i_mapping;
 		mapping[1] = inode1->i_mapping;
 	}
Signature · 1 line
2.7.4
maintainer
#1 lore raw
Quoted 7 lines · Gustavo A. R. Silva

Make use of the swap macro and remove unnecessary variable *tmp*. This makes the code easier to read and maintain. Also, reduces the stack usage.

This code was detected with the help of Coccinelle.

It doesn't actually reduce stack usage. Look at the definition of swap() in include/linux/kernel.h:

#define swap(a, b) \
	do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)

It removes the variable "tmp", and replace it with "__tmp".

It slightly reduces the number of lines of code, so it slightly makes the code easier to maintainer. But I think you are really over-promising how much this commit helps.

Regards,

- Ted

#2 lore raw

Hi Theodore,

Quoted 12 lines · Theodore Y. Ts'o
Quoted 7 lines · Gustavo A. R. Silva

Make use of the swap macro and remove unnecessary variable *tmp*. This makes the code easier to read and maintain. Also, reduces the stack usage.

This code was detected with the help of Coccinelle.

It doesn't actually reduce stack usage. Look at the definition of swap() in include/linux/kernel.h:

Quoted 9 lines
#define swap(a, b) \
	do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)

It removes the variable "tmp", and replace it with "__tmp".

It slightly reduces the number of lines of code, so it slightly makes the code easier to maintainer. But I think you are really over-promising how much this commit helps.

I'll remove that part from the changelog and send it again.

Thanks
--
Gustavo