[PATCH 0/4] Fix error path and use kernel macros for min/max/calmp

[PATCH 0/4] Fix error path and use kernel macros for min/max/calmp

[Kari Argillander]

First fix some error path fault which I found during when I did this.
It should go to 5.15.

For better code readability we can use kernel defined macros. Patch
2/4 also removes some dead code so that it would be more clear why
do we do what we do in patch 3/4.

Series is tested with xfs-tests. No recression in my tests.

Kari Argillander (4):
  fs/ntfs3: Fix ntfs_look_for_free_space() does only report -ENOSPC
  fs/ntfs3: Remove always false condition check
  fs/ntfs3: Use clamp/max macros instead of comparisons
  fs/ntfs3: Use min/max macros instated of ternary operators

 fs/ntfs3/attrib.c |  3 ++-
 fs/ntfs3/bitmap.c | 11 ++++----
 fs/ntfs3/fsntfs.c | 68 +++++++++++++++++++++++------------------------
 3 files changed, 41 insertions(+), 41 deletions(-)


base-commit: 2e3a51b59ea26544303e168de8a0479915f09aa3
-- 
2.25.1

[PATCH 1/4] fs/ntfs3: Fix ntfs_look_for_free_space() does only report -ENOSPC

[Kari Argillander]

If ntfs_refresh_zone() returns error it will be changed to -ENOSPC. It
is not right. Also caller of this functions also check other errors.

Fixes: 78ab59fee07f ("fs/ntfs3: Rework file operations")
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/fsntfs.c | 51 +++++++++++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 24 deletions(-)

diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index 91e3743e1442..ae7c6435d7bc 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -358,7 +358,7 @@ int ntfs_look_for_free_space(struct ntfs_sb_info *sbi, CLST lcn, CLST len,
 			     enum ALLOCATE_OPT opt)
 {
 	int err;
-	CLST alen = 0;
+	CLST alen;
 	struct super_block *sb = sbi->sb;
 	size_t alcn, zlen, zeroes, zlcn, zlen2, ztrim, new_zlen;
 	struct wnd_bitmap *wnd = &sbi->used.bitmap;
@@ -370,13 +370,15 @@ int ntfs_look_for_free_space(struct ntfs_sb_info *sbi, CLST lcn, CLST len,
 		if (!zlen) {
 			err = ntfs_refresh_zone(sbi);
 			if (err)
-				goto out;
+				goto up_write;
+
 			zlen = wnd_zone_len(wnd);
 		}
 
 		if (!zlen) {
 			ntfs_err(sbi->sb, "no free space to extend mft");
-			goto out;
+			err = -ENOSPC;
+			goto up_write;
 		}
 
 		lcn = wnd_zone_bit(wnd);
@@ -385,12 +387,11 @@ int ntfs_look_for_free_space(struct ntfs_sb_info *sbi, CLST lcn, CLST len,
 		wnd_zone_set(wnd, lcn + alen, zlen - alen);
 
 		err = wnd_set_used(wnd, lcn, alen);
-		if (err) {
-			up_write(&wnd->rw_lock);
-			return err;
-		}
+		if (err)
+			goto up_write;
+
 		alcn = lcn;
-		goto out;
+		goto space_found;
 	}
 	/*
 	 * 'Cause cluster 0 is always used this value means that we should use
@@ -404,15 +405,17 @@ int ntfs_look_for_free_space(struct ntfs_sb_info *sbi, CLST lcn, CLST len,
 
 	alen = wnd_find(wnd, len, lcn, BITMAP_FIND_MARK_AS_USED, &alcn);
 	if (alen)
-		goto out;
+		goto space_found;
 
 	/* Try to use clusters from MftZone. */
 	zlen = wnd_zone_len(wnd);
 	zeroes = wnd_zeroes(wnd);
 
 	/* Check too big request */
-	if (len > zeroes + zlen || zlen <= NTFS_MIN_MFT_ZONE)
-		goto out;
+	if (len > zeroes + zlen || zlen <= NTFS_MIN_MFT_ZONE) {
+		err = -ENOSPC;
+		goto up_write;
+	}
 
 	/* How many clusters to cat from zone. */
 	zlcn = wnd_zone_bit(wnd);
@@ -431,22 +434,22 @@ int ntfs_look_for_free_space(struct ntfs_sb_info *sbi, CLST lcn, CLST len,
 	/* Allocate continues clusters. */
 	alen = wnd_find(wnd, len, 0,
 			BITMAP_FIND_MARK_AS_USED | BITMAP_FIND_FULL, &alcn);
-
-out:
-	if (alen) {
-		err = 0;
-		*new_len = alen;
-		*new_lcn = alcn;
-
-		ntfs_unmap_meta(sb, alcn, alen);
-
-		/* Set hint for next requests. */
-		if (!(opt & ALLOCATE_MFT))
-			sbi->used.next_free_lcn = alcn + alen;
-	} else {
+	if (!alen) {
 		err = -ENOSPC;
+		goto up_write;
 	}
 
+space_found:
+	err = 0;
+	*new_len = alen;
+	*new_lcn = alcn;
+
+	ntfs_unmap_meta(sb, alcn, alen);
+
+	/* Set hint for next requests. */
+	if (!(opt & ALLOCATE_MFT))
+		sbi->used.next_free_lcn = alcn + alen;
+up_write:
 	up_write(&wnd->rw_lock);
 	return err;
 }
-- 
2.25.1

[PATCH 2/4] fs/ntfs3: Remove always false condition check

[Kari Argillander]

We do not need this check as this is same thing as
NTFS_MIN_MFT_ZONE > zlen. We already check NTFS_MIN_MFT_ZONE <= zlen and
exit because is too big request. Remove it so code is cleaner.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/fsntfs.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index ae7c6435d7bc..99d0a2799b0e 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -423,11 +423,8 @@ int ntfs_look_for_free_space(struct ntfs_sb_info *sbi, CLST lcn, CLST len,
 	ztrim = len > zlen ? zlen : (len > zlen2 ? len : zlen2);
 	new_zlen = zlen - ztrim;
 
-	if (new_zlen < NTFS_MIN_MFT_ZONE) {
+	if (new_zlen < NTFS_MIN_MFT_ZONE)
 		new_zlen = NTFS_MIN_MFT_ZONE;
-		if (new_zlen > zlen)
-			new_zlen = zlen;
-	}
 
 	wnd_zone_set(wnd, zlcn, new_zlen);
 
-- 
2.25.1

[PATCH 3/4] fs/ntfs3: Use clamp/max macros instead of comparisons

[Kari Argillander]

We can make code little more readable by using kernel macros clamp/max.

This were found with kernel included Coccinelle minmax script.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/fsntfs.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index 99d0a2799b0e..4a0a1aa085d5 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -8,6 +8,7 @@
 #include <linux/blkdev.h>
 #include <linux/buffer_head.h>
 #include <linux/fs.h>
+#include <linux/kernel.h>
 #include <linux/nls.h>
 
 #include "debug.h"
@@ -420,11 +421,8 @@ int ntfs_look_for_free_space(struct ntfs_sb_info *sbi, CLST lcn, CLST len,
 	/* How many clusters to cat from zone. */
 	zlcn = wnd_zone_bit(wnd);
 	zlen2 = zlen >> 1;
-	ztrim = len > zlen ? zlen : (len > zlen2 ? len : zlen2);
-	new_zlen = zlen - ztrim;
-
-	if (new_zlen < NTFS_MIN_MFT_ZONE)
-		new_zlen = NTFS_MIN_MFT_ZONE;
+	ztrim = clamp_val(len, zlen2, zlen);
+	new_zlen = max_t(size_t, zlen - ztrim, NTFS_MIN_MFT_ZONE);
 
 	wnd_zone_set(wnd, zlcn, new_zlen);
 
-- 
2.25.1

[PATCH 4/4] fs/ntfs3: Use min/max macros instated of ternary operators

[Kari Argillander]

We can make code little bit more readable by using min/max macros.

These were found with Coccinelle.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/attrib.c |  3 ++-
 fs/ntfs3/bitmap.c | 11 ++++++-----
 fs/ntfs3/fsntfs.c |  6 +++---
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
index 34c4cbf7e29b..89bfef9dc0c4 100644
--- a/fs/ntfs3/attrib.c
+++ b/fs/ntfs3/attrib.c
@@ -10,6 +10,7 @@
 #include <linux/buffer_head.h>
 #include <linux/fs.h>
 #include <linux/hash.h>
+#include <linux/kernel.h>
 #include <linux/nls.h>
 #include <linux/ratelimit.h>
 #include <linux/slab.h>
@@ -1966,7 +1967,7 @@ int attr_punch_hole(struct ntfs_inode *ni, u64 vbo, u64 bytes, u32 *frame_size)
 			return 0;
 
 		from = vbo;
-		to = (vbo + bytes) < data_size ? (vbo + bytes) : data_size;
+		to = min_t(u64, vbo + bytes, data_size);
 		memset(Add2Ptr(resident_data(attr_b), from), 0, to - from);
 		return 0;
 	}
diff --git a/fs/ntfs3/bitmap.c b/fs/ntfs3/bitmap.c
index 831501555009..c17121887e6a 100644
--- a/fs/ntfs3/bitmap.c
+++ b/fs/ntfs3/bitmap.c
@@ -13,6 +13,7 @@
 #include <linux/blkdev.h>
 #include <linux/buffer_head.h>
 #include <linux/fs.h>
+#include <linux/kernel.h>
 #include <linux/nls.h>
 
 #include "debug.h"
@@ -435,7 +436,7 @@ static void wnd_remove_free_ext(struct wnd_bitmap *wnd, size_t bit, size_t len)
 		;
 	} else {
 		n3 = rb_next(&e->count.node);
-		max_new_len = len > new_len ? len : new_len;
+		max_new_len = max(len, new_len);
 		if (!n3) {
 			wnd->extent_max = max_new_len;
 		} else {
@@ -731,7 +732,7 @@ int wnd_set_free(struct wnd_bitmap *wnd, size_t bit, size_t bits)
 			wbits = wnd->bits_last;
 
 		tail = wbits - wbit;
-		op = tail < bits ? tail : bits;
+		op = min_t(u32, tail, bits);
 
 		bh = wnd_map(wnd, iw);
 		if (IS_ERR(bh)) {
@@ -784,7 +785,7 @@ int wnd_set_used(struct wnd_bitmap *wnd, size_t bit, size_t bits)
 			wbits = wnd->bits_last;
 
 		tail = wbits - wbit;
-		op = tail < bits ? tail : bits;
+		op = min_t(u32, tail, bits);
 
 		bh = wnd_map(wnd, iw);
 		if (IS_ERR(bh)) {
@@ -834,7 +835,7 @@ static bool wnd_is_free_hlp(struct wnd_bitmap *wnd, size_t bit, size_t bits)
 			wbits = wnd->bits_last;
 
 		tail = wbits - wbit;
-		op = tail < bits ? tail : bits;
+		op = min_t(u32, tail, bits);
 
 		if (wbits != wnd->free_bits[iw]) {
 			bool ret;
@@ -926,7 +927,7 @@ bool wnd_is_used(struct wnd_bitmap *wnd, size_t bit, size_t bits)
 			wbits = wnd->bits_last;
 
 		tail = wbits - wbit;
-		op = tail < bits ? tail : bits;
+		op = min_t(u32, tail, bits);
 
 		if (wnd->free_bits[iw]) {
 			bool ret;
diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index 4a0a1aa085d5..77f52fe49dc1 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -383,7 +383,7 @@ int ntfs_look_for_free_space(struct ntfs_sb_info *sbi, CLST lcn, CLST len,
 		}
 
 		lcn = wnd_zone_bit(wnd);
-		alen = zlen > len ? len : zlen;
+		alen = min_t(CLST, len, zlen);
 
 		wnd_zone_set(wnd, lcn + alen, zlen - alen);
 
@@ -1097,7 +1097,7 @@ int ntfs_sb_write_run(struct ntfs_sb_info *sbi, const struct runs_tree *run,
 	len = ((u64)clen << cluster_bits) - off;
 
 	for (;;) {
-		u32 op = len < bytes ? len : bytes;
+		u32 op = min_t(u64, len, bytes);
 		int err = ntfs_sb_write(sb, lbo, op, buf, 0);
 
 		if (err)
@@ -1298,7 +1298,7 @@ int ntfs_get_bh(struct ntfs_sb_info *sbi, const struct runs_tree *run, u64 vbo,
 	nb->off = off = lbo & (blocksize - 1);
 
 	for (;;) {
-		u32 len32 = len < bytes ? len : bytes;
+		u32 len32 = min_t(u64, len, bytes);
 		sector_t block = lbo >> sb->s_blocksize_bits;
 
 		do {
-- 
2.25.1

Re: [PATCH 0/4] Fix error path and use kernel macros for min/max/calmp

[Konstantin Komarov]

On 07.09.2021 17:28, Kari Argillander wrote:
> First fix some error path fault which I found during when I did this.
> It should go to 5.15.
> 
> For better code readability we can use kernel defined macros. Patch
> 2/4 also removes some dead code so that it would be more clear why
> do we do what we do in patch 3/4.
> 
> Series is tested with xfs-tests. No recression in my tests.
> 
> Kari Argillander (4):
>   fs/ntfs3: Fix ntfs_look_for_free_space() does only report -ENOSPC
>   fs/ntfs3: Remove always false condition check
>   fs/ntfs3: Use clamp/max macros instead of comparisons
>   fs/ntfs3: Use min/max macros instated of ternary operators
> 
>  fs/ntfs3/attrib.c |  3 ++-
>  fs/ntfs3/bitmap.c | 11 ++++----
>  fs/ntfs3/fsntfs.c | 68 +++++++++++++++++++++++------------------------
>  3 files changed, 41 insertions(+), 41 deletions(-)
> 
> 
> base-commit: 2e3a51b59ea26544303e168de8a0479915f09aa3
> 

Thanks for patches, applied!