* [PATCH 1/11] [PATCH] xfs: rename diff_two_keys routines
2025-10-08 16:41 [PATCH 0/12] xfsprogs: libxfs sync v6.17 Andrey Albershteyn
@ 2025-10-08 16:53 ` Fedor Pchelkin
2025-10-08 16:54 ` [PATCH 2/11] [PATCH] xfs: rename key_diff routines Fedor Pchelkin
` (10 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Fedor Pchelkin @ 2025-10-08 16:53 UTC (permalink / raw)
To: linux-xfs, aalbersh, cem, cmaiolino, djwong, hch, pchelkin,
pranav.tyagi03, sandeen
Source kernel commit: edce172444b4f489715a3df2e5d50893e74ce3da
One may think that diff_two_keys routines are used to compute the actual
difference between the arguments but they return a result of a
three-way-comparison of the passed operands. So it looks more appropriate
to denote them as cmp_two_keys.
Found by Linux Verification Center (linuxtesting.org).
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
libxfs/xfs_alloc_btree.c | 8 ++++----
libxfs/xfs_bmap_btree.c | 4 ++--
libxfs/xfs_btree.c | 2 +-
libxfs/xfs_btree.h | 26 +++++++++++++-------------
libxfs/xfs_ialloc_btree.c | 6 +++---
libxfs/xfs_refcount_btree.c | 4 ++--
libxfs/xfs_rmap_btree.c | 6 +++---
libxfs/xfs_rtrefcount_btree.c | 4 ++--
libxfs/xfs_rtrmap_btree.c | 6 +++---
repair/rcbag_btree.c | 4 ++--
10 files changed, 35 insertions(+), 35 deletions(-)
diff --git a/libxfs/xfs_alloc_btree.c b/libxfs/xfs_alloc_btree.c
index 63b8db4ed0..85ce9f728f 100644
--- a/libxfs/xfs_alloc_btree.c
+++ b/libxfs/xfs_alloc_btree.c
@@ -212,7 +212,7 @@
}
STATIC int64_t
-xfs_bnobt_diff_two_keys(
+xfs_bnobt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
const union xfs_btree_key *k2,
@@ -225,7 +225,7 @@
}
STATIC int64_t
-xfs_cntbt_diff_two_keys(
+xfs_cntbt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
const union xfs_btree_key *k2,
@@ -438,7 +438,7 @@
.init_ptr_from_cur = xfs_allocbt_init_ptr_from_cur,
.key_diff = xfs_bnobt_key_diff,
.buf_ops = &xfs_bnobt_buf_ops,
- .diff_two_keys = xfs_bnobt_diff_two_keys,
+ .cmp_two_keys = xfs_bnobt_cmp_two_keys,
.keys_inorder = xfs_bnobt_keys_inorder,
.recs_inorder = xfs_bnobt_recs_inorder,
.keys_contiguous = xfs_allocbt_keys_contiguous,
@@ -468,7 +468,7 @@
.init_ptr_from_cur = xfs_allocbt_init_ptr_from_cur,
.key_diff = xfs_cntbt_key_diff,
.buf_ops = &xfs_cntbt_buf_ops,
- .diff_two_keys = xfs_cntbt_diff_two_keys,
+ .cmp_two_keys = xfs_cntbt_cmp_two_keys,
.keys_inorder = xfs_cntbt_keys_inorder,
.recs_inorder = xfs_cntbt_recs_inorder,
.keys_contiguous = NULL, /* not needed right now */
diff --git a/libxfs/xfs_bmap_btree.c b/libxfs/xfs_bmap_btree.c
index 40e0cb3016..eb3e5d70de 100644
--- a/libxfs/xfs_bmap_btree.c
+++ b/libxfs/xfs_bmap_btree.c
@@ -378,7 +378,7 @@
}
STATIC int64_t
-xfs_bmbt_diff_two_keys(
+xfs_bmbt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
const union xfs_btree_key *k2,
@@ -647,7 +647,7 @@
.init_high_key_from_rec = xfs_bmbt_init_high_key_from_rec,
.init_rec_from_cur = xfs_bmbt_init_rec_from_cur,
.key_diff = xfs_bmbt_key_diff,
- .diff_two_keys = xfs_bmbt_diff_two_keys,
+ .cmp_two_keys = xfs_bmbt_cmp_two_keys,
.buf_ops = &xfs_bmbt_buf_ops,
.keys_inorder = xfs_bmbt_keys_inorder,
.recs_inorder = xfs_bmbt_recs_inorder,
diff --git a/libxfs/xfs_btree.c b/libxfs/xfs_btree.c
index fce215f924..78cc31f61e 100644
--- a/libxfs/xfs_btree.c
+++ b/libxfs/xfs_btree.c
@@ -5055,7 +5055,7 @@
int error;
ASSERT(cur->bc_ops->init_high_key_from_rec);
- ASSERT(cur->bc_ops->diff_two_keys);
+ ASSERT(cur->bc_ops->cmp_two_keys);
/*
* Find the leftmost record. The btree cursor must be set
diff --git a/libxfs/xfs_btree.h b/libxfs/xfs_btree.h
index 355b304696..1046bbf383 100644
--- a/libxfs/xfs_btree.h
+++ b/libxfs/xfs_btree.h
@@ -176,15 +176,15 @@
const union xfs_btree_key *key);
/*
- * Difference between key2 and key1 -- positive if key1 > key2,
- * negative if key1 < key2, and zero if equal. If the @mask parameter
- * is non NULL, each key field to be used in the comparison must
- * contain a nonzero value.
+ * Compare key1 and key2 -- positive if key1 > key2, negative if
+ * key1 < key2, and zero if equal. If the @mask parameter is non NULL,
+ * each key field to be used in the comparison must contain a nonzero
+ * value.
*/
- int64_t (*diff_two_keys)(struct xfs_btree_cur *cur,
- const union xfs_btree_key *key1,
- const union xfs_btree_key *key2,
- const union xfs_btree_key *mask);
+ int64_t (*cmp_two_keys)(struct xfs_btree_cur *cur,
+ const union xfs_btree_key *key1,
+ const union xfs_btree_key *key2,
+ const union xfs_btree_key *mask);
const struct xfs_buf_ops *buf_ops;
@@ -546,7 +546,7 @@
const union xfs_btree_key *key1,
const union xfs_btree_key *key2)
{
- return cur->bc_ops->diff_two_keys(cur, key1, key2, NULL) < 0;
+ return cur->bc_ops->cmp_two_keys(cur, key1, key2, NULL) < 0;
}
static inline bool
@@ -555,7 +555,7 @@
const union xfs_btree_key *key1,
const union xfs_btree_key *key2)
{
- return cur->bc_ops->diff_two_keys(cur, key1, key2, NULL) > 0;
+ return cur->bc_ops->cmp_two_keys(cur, key1, key2, NULL) > 0;
}
static inline bool
@@ -564,7 +564,7 @@
const union xfs_btree_key *key1,
const union xfs_btree_key *key2)
{
- return cur->bc_ops->diff_two_keys(cur, key1, key2, NULL) == 0;
+ return cur->bc_ops->cmp_two_keys(cur, key1, key2, NULL) == 0;
}
static inline bool
@@ -602,7 +602,7 @@
const union xfs_btree_key *key2,
const union xfs_btree_key *mask)
{
- return cur->bc_ops->diff_two_keys(cur, key1, key2, mask) < 0;
+ return cur->bc_ops->cmp_two_keys(cur, key1, key2, mask) < 0;
}
static inline bool
@@ -612,7 +612,7 @@
const union xfs_btree_key *key2,
const union xfs_btree_key *mask)
{
- return cur->bc_ops->diff_two_keys(cur, key1, key2, mask) > 0;
+ return cur->bc_ops->cmp_two_keys(cur, key1, key2, mask) > 0;
}
static inline bool
diff --git a/libxfs/xfs_ialloc_btree.c b/libxfs/xfs_ialloc_btree.c
index 4cccac145d..d5f55f7466 100644
--- a/libxfs/xfs_ialloc_btree.c
+++ b/libxfs/xfs_ialloc_btree.c
@@ -274,7 +274,7 @@
}
STATIC int64_t
-xfs_inobt_diff_two_keys(
+xfs_inobt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
const union xfs_btree_key *k2,
@@ -431,7 +431,7 @@
.init_ptr_from_cur = xfs_inobt_init_ptr_from_cur,
.key_diff = xfs_inobt_key_diff,
.buf_ops = &xfs_inobt_buf_ops,
- .diff_two_keys = xfs_inobt_diff_two_keys,
+ .cmp_two_keys = xfs_inobt_cmp_two_keys,
.keys_inorder = xfs_inobt_keys_inorder,
.recs_inorder = xfs_inobt_recs_inorder,
.keys_contiguous = xfs_inobt_keys_contiguous,
@@ -461,7 +461,7 @@
.init_ptr_from_cur = xfs_finobt_init_ptr_from_cur,
.key_diff = xfs_inobt_key_diff,
.buf_ops = &xfs_finobt_buf_ops,
- .diff_two_keys = xfs_inobt_diff_two_keys,
+ .cmp_two_keys = xfs_inobt_cmp_two_keys,
.keys_inorder = xfs_inobt_keys_inorder,
.recs_inorder = xfs_inobt_recs_inorder,
.keys_contiguous = xfs_inobt_keys_contiguous,
diff --git a/libxfs/xfs_refcount_btree.c b/libxfs/xfs_refcount_btree.c
index efb9af710f..63417b873c 100644
--- a/libxfs/xfs_refcount_btree.c
+++ b/libxfs/xfs_refcount_btree.c
@@ -188,7 +188,7 @@
}
STATIC int64_t
-xfs_refcountbt_diff_two_keys(
+xfs_refcountbt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
const union xfs_btree_key *k2,
@@ -340,7 +340,7 @@
.init_ptr_from_cur = xfs_refcountbt_init_ptr_from_cur,
.key_diff = xfs_refcountbt_key_diff,
.buf_ops = &xfs_refcountbt_buf_ops,
- .diff_two_keys = xfs_refcountbt_diff_two_keys,
+ .cmp_two_keys = xfs_refcountbt_cmp_two_keys,
.keys_inorder = xfs_refcountbt_keys_inorder,
.recs_inorder = xfs_refcountbt_recs_inorder,
.keys_contiguous = xfs_refcountbt_keys_contiguous,
diff --git a/libxfs/xfs_rmap_btree.c b/libxfs/xfs_rmap_btree.c
index f4da35050d..0481850e46 100644
--- a/libxfs/xfs_rmap_btree.c
+++ b/libxfs/xfs_rmap_btree.c
@@ -273,7 +273,7 @@
}
STATIC int64_t
-xfs_rmapbt_diff_two_keys(
+xfs_rmapbt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
const union xfs_btree_key *k2,
@@ -516,7 +516,7 @@
.init_ptr_from_cur = xfs_rmapbt_init_ptr_from_cur,
.key_diff = xfs_rmapbt_key_diff,
.buf_ops = &xfs_rmapbt_buf_ops,
- .diff_two_keys = xfs_rmapbt_diff_two_keys,
+ .cmp_two_keys = xfs_rmapbt_cmp_two_keys,
.keys_inorder = xfs_rmapbt_keys_inorder,
.recs_inorder = xfs_rmapbt_recs_inorder,
.keys_contiguous = xfs_rmapbt_keys_contiguous,
@@ -633,7 +633,7 @@
.init_ptr_from_cur = xfbtree_init_ptr_from_cur,
.key_diff = xfs_rmapbt_key_diff,
.buf_ops = &xfs_rmapbt_mem_buf_ops,
- .diff_two_keys = xfs_rmapbt_diff_two_keys,
+ .cmp_two_keys = xfs_rmapbt_cmp_two_keys,
.keys_inorder = xfs_rmapbt_keys_inorder,
.recs_inorder = xfs_rmapbt_recs_inorder,
.keys_contiguous = xfs_rmapbt_keys_contiguous,
diff --git a/libxfs/xfs_rtrefcount_btree.c b/libxfs/xfs_rtrefcount_btree.c
index a532476407..1f563a724c 100644
--- a/libxfs/xfs_rtrefcount_btree.c
+++ b/libxfs/xfs_rtrefcount_btree.c
@@ -169,7 +169,7 @@
}
STATIC int64_t
-xfs_rtrefcountbt_diff_two_keys(
+xfs_rtrefcountbt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
const union xfs_btree_key *k2,
@@ -387,7 +387,7 @@
.init_ptr_from_cur = xfs_rtrefcountbt_init_ptr_from_cur,
.key_diff = xfs_rtrefcountbt_key_diff,
.buf_ops = &xfs_rtrefcountbt_buf_ops,
- .diff_two_keys = xfs_rtrefcountbt_diff_two_keys,
+ .cmp_two_keys = xfs_rtrefcountbt_cmp_two_keys,
.keys_inorder = xfs_rtrefcountbt_keys_inorder,
.recs_inorder = xfs_rtrefcountbt_recs_inorder,
.keys_contiguous = xfs_rtrefcountbt_keys_contiguous,
diff --git a/libxfs/xfs_rtrmap_btree.c b/libxfs/xfs_rtrmap_btree.c
index d46fb67bb5..5b3836fccc 100644
--- a/libxfs/xfs_rtrmap_btree.c
+++ b/libxfs/xfs_rtrmap_btree.c
@@ -215,7 +215,7 @@
}
STATIC int64_t
-xfs_rtrmapbt_diff_two_keys(
+xfs_rtrmapbt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
const union xfs_btree_key *k2,
@@ -512,7 +512,7 @@
.init_ptr_from_cur = xfs_rtrmapbt_init_ptr_from_cur,
.key_diff = xfs_rtrmapbt_key_diff,
.buf_ops = &xfs_rtrmapbt_buf_ops,
- .diff_two_keys = xfs_rtrmapbt_diff_two_keys,
+ .cmp_two_keys = xfs_rtrmapbt_cmp_two_keys,
.keys_inorder = xfs_rtrmapbt_keys_inorder,
.recs_inorder = xfs_rtrmapbt_recs_inorder,
.keys_contiguous = xfs_rtrmapbt_keys_contiguous,
@@ -621,7 +621,7 @@
.init_ptr_from_cur = xfbtree_init_ptr_from_cur,
.key_diff = xfs_rtrmapbt_key_diff,
.buf_ops = &xfs_rtrmapbt_mem_buf_ops,
- .diff_two_keys = xfs_rtrmapbt_diff_two_keys,
+ .cmp_two_keys = xfs_rtrmapbt_cmp_two_keys,
.keys_inorder = xfs_rtrmapbt_keys_inorder,
.recs_inorder = xfs_rtrmapbt_recs_inorder,
.keys_contiguous = xfs_rtrmapbt_keys_contiguous,
diff --git a/repair/rcbag_btree.c b/repair/rcbag_btree.c
index bed7c7a8f6..404b509f64 100644
--- a/repair/rcbag_btree.c
+++ b/repair/rcbag_btree.c
@@ -73,7 +73,7 @@
}
STATIC int64_t
-rcbagbt_diff_two_keys(
+rcbagbt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
const union xfs_btree_key *k2,
@@ -222,7 +222,7 @@
.init_ptr_from_cur = xfbtree_init_ptr_from_cur,
.key_diff = rcbagbt_key_diff,
.buf_ops = &rcbagbt_mem_buf_ops,
- .diff_two_keys = rcbagbt_diff_two_keys,
+ .cmp_two_keys = rcbagbt_cmp_two_keys,
.keys_inorder = rcbagbt_keys_inorder,
.recs_inorder = rcbagbt_recs_inorder,
};
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/11] [PATCH] xfs: rename key_diff routines
2025-10-08 16:41 [PATCH 0/12] xfsprogs: libxfs sync v6.17 Andrey Albershteyn
2025-10-08 16:53 ` [PATCH 1/11] [PATCH] xfs: rename diff_two_keys routines Fedor Pchelkin
@ 2025-10-08 16:54 ` Fedor Pchelkin
2025-10-08 16:55 ` [PATCH 3/11] [PATCH] xfs: refactor cmp_two_keys routines to take advantage of cmp_int() Fedor Pchelkin
` (9 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Fedor Pchelkin @ 2025-10-08 16:54 UTC (permalink / raw)
To: linux-xfs, aalbersh, cem, cmaiolino, djwong, hch, pchelkin,
pranav.tyagi03, sandeen
Source kernel commit: 82b63ee160016096436aa026a27c8d85d40f3fb1
key_diff routines compare a key value with a cursor value. Make the naming
to be a bit more self-descriptive.
Found by Linux Verification Center (linuxtesting.org).
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
libxfs/xfs_alloc_btree.c | 8 ++++----
libxfs/xfs_bmap_btree.c | 4 ++--
libxfs/xfs_btree.c | 2 +-
libxfs/xfs_btree.h | 9 ++++++---
libxfs/xfs_ialloc_btree.c | 6 +++---
libxfs/xfs_refcount_btree.c | 4 ++--
libxfs/xfs_rmap_btree.c | 6 +++---
libxfs/xfs_rtrefcount_btree.c | 4 ++--
libxfs/xfs_rtrmap_btree.c | 6 +++---
repair/rcbag_btree.c | 4 ++--
10 files changed, 28 insertions(+), 25 deletions(-)
diff --git a/libxfs/xfs_alloc_btree.c b/libxfs/xfs_alloc_btree.c
index 85ce9f728f..6e7af0020b 100644
--- a/libxfs/xfs_alloc_btree.c
+++ b/libxfs/xfs_alloc_btree.c
@@ -185,7 +185,7 @@
}
STATIC int64_t
-xfs_bnobt_key_diff(
+xfs_bnobt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
{
@@ -196,7 +196,7 @@
}
STATIC int64_t
-xfs_cntbt_key_diff(
+xfs_cntbt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
{
@@ -436,7 +436,7 @@
.init_high_key_from_rec = xfs_bnobt_init_high_key_from_rec,
.init_rec_from_cur = xfs_allocbt_init_rec_from_cur,
.init_ptr_from_cur = xfs_allocbt_init_ptr_from_cur,
- .key_diff = xfs_bnobt_key_diff,
+ .cmp_key_with_cur = xfs_bnobt_cmp_key_with_cur,
.buf_ops = &xfs_bnobt_buf_ops,
.cmp_two_keys = xfs_bnobt_cmp_two_keys,
.keys_inorder = xfs_bnobt_keys_inorder,
@@ -466,7 +466,7 @@
.init_high_key_from_rec = xfs_cntbt_init_high_key_from_rec,
.init_rec_from_cur = xfs_allocbt_init_rec_from_cur,
.init_ptr_from_cur = xfs_allocbt_init_ptr_from_cur,
- .key_diff = xfs_cntbt_key_diff,
+ .cmp_key_with_cur = xfs_cntbt_cmp_key_with_cur,
.buf_ops = &xfs_cntbt_buf_ops,
.cmp_two_keys = xfs_cntbt_cmp_two_keys,
.keys_inorder = xfs_cntbt_keys_inorder,
diff --git a/libxfs/xfs_bmap_btree.c b/libxfs/xfs_bmap_btree.c
index eb3e5d70de..3fc23444f3 100644
--- a/libxfs/xfs_bmap_btree.c
+++ b/libxfs/xfs_bmap_btree.c
@@ -369,7 +369,7 @@
}
STATIC int64_t
-xfs_bmbt_key_diff(
+xfs_bmbt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
{
@@ -646,7 +646,7 @@
.init_key_from_rec = xfs_bmbt_init_key_from_rec,
.init_high_key_from_rec = xfs_bmbt_init_high_key_from_rec,
.init_rec_from_cur = xfs_bmbt_init_rec_from_cur,
- .key_diff = xfs_bmbt_key_diff,
+ .cmp_key_with_cur = xfs_bmbt_cmp_key_with_cur,
.cmp_two_keys = xfs_bmbt_cmp_two_keys,
.buf_ops = &xfs_bmbt_buf_ops,
.keys_inorder = xfs_bmbt_keys_inorder,
diff --git a/libxfs/xfs_btree.c b/libxfs/xfs_btree.c
index 78cc31f61e..15846f0ff6 100644
--- a/libxfs/xfs_btree.c
+++ b/libxfs/xfs_btree.c
@@ -2067,7 +2067,7 @@
* - greater than, move left
* - equal, we're done
*/
- diff = cur->bc_ops->key_diff(cur, kp);
+ diff = cur->bc_ops->cmp_key_with_cur(cur, kp);
if (diff < 0)
low = keyno + 1;
else if (diff > 0)
diff --git a/libxfs/xfs_btree.h b/libxfs/xfs_btree.h
index 1046bbf383..e72a10ba7e 100644
--- a/libxfs/xfs_btree.h
+++ b/libxfs/xfs_btree.h
@@ -171,9 +171,12 @@
void (*init_high_key_from_rec)(union xfs_btree_key *key,
const union xfs_btree_rec *rec);
- /* difference between key value and cursor value */
- int64_t (*key_diff)(struct xfs_btree_cur *cur,
- const union xfs_btree_key *key);
+ /*
+ * Compare key value and cursor value -- positive if key > cur,
+ * negative if key < cur, and zero if equal.
+ */
+ int64_t (*cmp_key_with_cur)(struct xfs_btree_cur *cur,
+ const union xfs_btree_key *key);
/*
* Compare key1 and key2 -- positive if key1 > key2, negative if
diff --git a/libxfs/xfs_ialloc_btree.c b/libxfs/xfs_ialloc_btree.c
index d5f55f7466..d56876c5be 100644
--- a/libxfs/xfs_ialloc_btree.c
+++ b/libxfs/xfs_ialloc_btree.c
@@ -265,7 +265,7 @@
}
STATIC int64_t
-xfs_inobt_key_diff(
+xfs_inobt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
{
@@ -429,7 +429,7 @@
.init_high_key_from_rec = xfs_inobt_init_high_key_from_rec,
.init_rec_from_cur = xfs_inobt_init_rec_from_cur,
.init_ptr_from_cur = xfs_inobt_init_ptr_from_cur,
- .key_diff = xfs_inobt_key_diff,
+ .cmp_key_with_cur = xfs_inobt_cmp_key_with_cur,
.buf_ops = &xfs_inobt_buf_ops,
.cmp_two_keys = xfs_inobt_cmp_two_keys,
.keys_inorder = xfs_inobt_keys_inorder,
@@ -459,7 +459,7 @@
.init_high_key_from_rec = xfs_inobt_init_high_key_from_rec,
.init_rec_from_cur = xfs_inobt_init_rec_from_cur,
.init_ptr_from_cur = xfs_finobt_init_ptr_from_cur,
- .key_diff = xfs_inobt_key_diff,
+ .cmp_key_with_cur = xfs_inobt_cmp_key_with_cur,
.buf_ops = &xfs_finobt_buf_ops,
.cmp_two_keys = xfs_inobt_cmp_two_keys,
.keys_inorder = xfs_inobt_keys_inorder,
diff --git a/libxfs/xfs_refcount_btree.c b/libxfs/xfs_refcount_btree.c
index 63417b873c..0924ab7eb7 100644
--- a/libxfs/xfs_refcount_btree.c
+++ b/libxfs/xfs_refcount_btree.c
@@ -174,7 +174,7 @@
}
STATIC int64_t
-xfs_refcountbt_key_diff(
+xfs_refcountbt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
{
@@ -338,7 +338,7 @@
.init_high_key_from_rec = xfs_refcountbt_init_high_key_from_rec,
.init_rec_from_cur = xfs_refcountbt_init_rec_from_cur,
.init_ptr_from_cur = xfs_refcountbt_init_ptr_from_cur,
- .key_diff = xfs_refcountbt_key_diff,
+ .cmp_key_with_cur = xfs_refcountbt_cmp_key_with_cur,
.buf_ops = &xfs_refcountbt_buf_ops,
.cmp_two_keys = xfs_refcountbt_cmp_two_keys,
.keys_inorder = xfs_refcountbt_keys_inorder,
diff --git a/libxfs/xfs_rmap_btree.c b/libxfs/xfs_rmap_btree.c
index 0481850e46..ea946616bf 100644
--- a/libxfs/xfs_rmap_btree.c
+++ b/libxfs/xfs_rmap_btree.c
@@ -243,7 +243,7 @@
}
STATIC int64_t
-xfs_rmapbt_key_diff(
+xfs_rmapbt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
{
@@ -514,7 +514,7 @@
.init_high_key_from_rec = xfs_rmapbt_init_high_key_from_rec,
.init_rec_from_cur = xfs_rmapbt_init_rec_from_cur,
.init_ptr_from_cur = xfs_rmapbt_init_ptr_from_cur,
- .key_diff = xfs_rmapbt_key_diff,
+ .cmp_key_with_cur = xfs_rmapbt_cmp_key_with_cur,
.buf_ops = &xfs_rmapbt_buf_ops,
.cmp_two_keys = xfs_rmapbt_cmp_two_keys,
.keys_inorder = xfs_rmapbt_keys_inorder,
@@ -631,7 +631,7 @@
.init_high_key_from_rec = xfs_rmapbt_init_high_key_from_rec,
.init_rec_from_cur = xfs_rmapbt_init_rec_from_cur,
.init_ptr_from_cur = xfbtree_init_ptr_from_cur,
- .key_diff = xfs_rmapbt_key_diff,
+ .cmp_key_with_cur = xfs_rmapbt_cmp_key_with_cur,
.buf_ops = &xfs_rmapbt_mem_buf_ops,
.cmp_two_keys = xfs_rmapbt_cmp_two_keys,
.keys_inorder = xfs_rmapbt_keys_inorder,
diff --git a/libxfs/xfs_rtrefcount_btree.c b/libxfs/xfs_rtrefcount_btree.c
index 1f563a724c..7a4eec49ca 100644
--- a/libxfs/xfs_rtrefcount_btree.c
+++ b/libxfs/xfs_rtrefcount_btree.c
@@ -155,7 +155,7 @@
}
STATIC int64_t
-xfs_rtrefcountbt_key_diff(
+xfs_rtrefcountbt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
{
@@ -385,7 +385,7 @@
.init_high_key_from_rec = xfs_rtrefcountbt_init_high_key_from_rec,
.init_rec_from_cur = xfs_rtrefcountbt_init_rec_from_cur,
.init_ptr_from_cur = xfs_rtrefcountbt_init_ptr_from_cur,
- .key_diff = xfs_rtrefcountbt_key_diff,
+ .cmp_key_with_cur = xfs_rtrefcountbt_cmp_key_with_cur,
.buf_ops = &xfs_rtrefcountbt_buf_ops,
.cmp_two_keys = xfs_rtrefcountbt_cmp_two_keys,
.keys_inorder = xfs_rtrefcountbt_keys_inorder,
diff --git a/libxfs/xfs_rtrmap_btree.c b/libxfs/xfs_rtrmap_btree.c
index 5b3836fccc..59a99bb42c 100644
--- a/libxfs/xfs_rtrmap_btree.c
+++ b/libxfs/xfs_rtrmap_btree.c
@@ -185,7 +185,7 @@
}
STATIC int64_t
-xfs_rtrmapbt_key_diff(
+xfs_rtrmapbt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
{
@@ -510,7 +510,7 @@
.init_high_key_from_rec = xfs_rtrmapbt_init_high_key_from_rec,
.init_rec_from_cur = xfs_rtrmapbt_init_rec_from_cur,
.init_ptr_from_cur = xfs_rtrmapbt_init_ptr_from_cur,
- .key_diff = xfs_rtrmapbt_key_diff,
+ .cmp_key_with_cur = xfs_rtrmapbt_cmp_key_with_cur,
.buf_ops = &xfs_rtrmapbt_buf_ops,
.cmp_two_keys = xfs_rtrmapbt_cmp_two_keys,
.keys_inorder = xfs_rtrmapbt_keys_inorder,
@@ -619,7 +619,7 @@
.init_high_key_from_rec = xfs_rtrmapbt_init_high_key_from_rec,
.init_rec_from_cur = xfs_rtrmapbt_init_rec_from_cur,
.init_ptr_from_cur = xfbtree_init_ptr_from_cur,
- .key_diff = xfs_rtrmapbt_key_diff,
+ .cmp_key_with_cur = xfs_rtrmapbt_cmp_key_with_cur,
.buf_ops = &xfs_rtrmapbt_mem_buf_ops,
.cmp_two_keys = xfs_rtrmapbt_cmp_two_keys,
.keys_inorder = xfs_rtrmapbt_keys_inorder,
diff --git a/repair/rcbag_btree.c b/repair/rcbag_btree.c
index 404b509f64..704e66e9fb 100644
--- a/repair/rcbag_btree.c
+++ b/repair/rcbag_btree.c
@@ -47,7 +47,7 @@
}
STATIC int64_t
-rcbagbt_key_diff(
+rcbagbt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
{
@@ -220,7 +220,7 @@
.init_key_from_rec = rcbagbt_init_key_from_rec,
.init_rec_from_cur = rcbagbt_init_rec_from_cur,
.init_ptr_from_cur = xfbtree_init_ptr_from_cur,
- .key_diff = rcbagbt_key_diff,
+ .cmp_key_with_cur = rcbagbt_cmp_key_with_cur,
.buf_ops = &rcbagbt_mem_buf_ops,
.cmp_two_keys = rcbagbt_cmp_two_keys,
.keys_inorder = rcbagbt_keys_inorder,
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/11] [PATCH] xfs: refactor cmp_two_keys routines to take advantage of cmp_int()
2025-10-08 16:41 [PATCH 0/12] xfsprogs: libxfs sync v6.17 Andrey Albershteyn
2025-10-08 16:53 ` [PATCH 1/11] [PATCH] xfs: rename diff_two_keys routines Fedor Pchelkin
2025-10-08 16:54 ` [PATCH 2/11] [PATCH] xfs: rename key_diff routines Fedor Pchelkin
@ 2025-10-08 16:55 ` Fedor Pchelkin
2025-10-08 20:36 ` Darrick J. Wong
2025-10-08 16:55 ` [PATCH 4/11] [PATCH] xfs: refactor cmp_key_with_cur " Fedor Pchelkin
` (8 subsequent siblings)
11 siblings, 1 reply; 15+ messages in thread
From: Fedor Pchelkin @ 2025-10-08 16:55 UTC (permalink / raw)
To: linux-xfs, aalbersh, cem, cmaiolino, djwong, hch, pchelkin,
pranav.tyagi03, sandeen
Source kernel commit: 3b583adf55c649d5ba37bcd1ca87644b0bc10b86
The net value of these functions is to determine the result of a
three-way-comparison between operands of the same type.
Simplify the code using cmp_int() to eliminate potential errors with
opencoded casts and subtractions. This also means we can change the return
value type of cmp_two_keys routines from int64_t to int and make the
interface a bit clearer.
Found by Linux Verification Center (linuxtesting.org).
Suggested-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
include/platform_defs.h | 2 ++
libxfs/xfs_alloc_btree.c | 21 ++++++++-------------
libxfs/xfs_bmap_btree.c | 18 +++---------------
libxfs/xfs_btree.h | 2 +-
libxfs/xfs_ialloc_btree.c | 6 +++---
libxfs/xfs_refcount_btree.c | 6 +++---
libxfs/xfs_rmap_btree.c | 29 ++++++++++++-----------------
libxfs/xfs_rtrefcount_btree.c | 6 +++---
libxfs/xfs_rtrmap_btree.c | 29 ++++++++++++-----------------
repair/rcbag_btree.c | 2 +-
scrub/inodes.c | 2 --
11 files changed, 48 insertions(+), 75 deletions(-)
diff --git a/include/platform_defs.h b/include/platform_defs.h
index 74a00583eb..fa66551d99 100644
--- a/include/platform_defs.h
+++ b/include/platform_defs.h
@@ -294,4 +294,6 @@
__a > __b ? (__a - __b) : (__b - __a); \
})
+#define cmp_int(l, r) ((l > r) - (l < r))
+
#endif /* __XFS_PLATFORM_DEFS_H__ */
diff --git a/libxfs/xfs_alloc_btree.c b/libxfs/xfs_alloc_btree.c
index 6e7af0020b..c3c69a9de3 100644
--- a/libxfs/xfs_alloc_btree.c
+++ b/libxfs/xfs_alloc_btree.c
@@ -211,7 +211,7 @@
return (int64_t)be32_to_cpu(kp->ar_startblock) - rec->ar_startblock;
}
-STATIC int64_t
+STATIC int
xfs_bnobt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
@@ -220,29 +220,24 @@
{
ASSERT(!mask || mask->alloc.ar_startblock);
- return (int64_t)be32_to_cpu(k1->alloc.ar_startblock) -
- be32_to_cpu(k2->alloc.ar_startblock);
+ return cmp_int(be32_to_cpu(k1->alloc.ar_startblock),
+ be32_to_cpu(k2->alloc.ar_startblock));
}
-STATIC int64_t
+STATIC int
xfs_cntbt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
const union xfs_btree_key *k2,
const union xfs_btree_key *mask)
{
- int64_t diff;
-
ASSERT(!mask || (mask->alloc.ar_blockcount &&
mask->alloc.ar_startblock));
- diff = be32_to_cpu(k1->alloc.ar_blockcount) -
- be32_to_cpu(k2->alloc.ar_blockcount);
- if (diff)
- return diff;
-
- return be32_to_cpu(k1->alloc.ar_startblock) -
- be32_to_cpu(k2->alloc.ar_startblock);
+ return cmp_int(be32_to_cpu(k1->alloc.ar_blockcount),
+ be32_to_cpu(k2->alloc.ar_blockcount)) ?:
+ cmp_int(be32_to_cpu(k1->alloc.ar_startblock),
+ be32_to_cpu(k2->alloc.ar_startblock));
}
static xfs_failaddr_t
diff --git a/libxfs/xfs_bmap_btree.c b/libxfs/xfs_bmap_btree.c
index 3fc23444f3..19eab66fad 100644
--- a/libxfs/xfs_bmap_btree.c
+++ b/libxfs/xfs_bmap_btree.c
@@ -377,29 +377,17 @@
cur->bc_rec.b.br_startoff;
}
-STATIC int64_t
+STATIC int
xfs_bmbt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
const union xfs_btree_key *k2,
const union xfs_btree_key *mask)
{
- uint64_t a = be64_to_cpu(k1->bmbt.br_startoff);
- uint64_t b = be64_to_cpu(k2->bmbt.br_startoff);
-
ASSERT(!mask || mask->bmbt.br_startoff);
- /*
- * Note: This routine previously casted a and b to int64 and subtracted
- * them to generate a result. This lead to problems if b was the
- * "maximum" key value (all ones) being signed incorrectly, hence this
- * somewhat less efficient version.
- */
- if (a > b)
- return 1;
- if (b > a)
- return -1;
- return 0;
+ return cmp_int(be64_to_cpu(k1->bmbt.br_startoff),
+ be64_to_cpu(k2->bmbt.br_startoff));
}
static xfs_failaddr_t
diff --git a/libxfs/xfs_btree.h b/libxfs/xfs_btree.h
index e72a10ba7e..fecd9f0b93 100644
--- a/libxfs/xfs_btree.h
+++ b/libxfs/xfs_btree.h
@@ -184,7 +184,7 @@
* each key field to be used in the comparison must contain a nonzero
* value.
*/
- int64_t (*cmp_two_keys)(struct xfs_btree_cur *cur,
+ int (*cmp_two_keys)(struct xfs_btree_cur *cur,
const union xfs_btree_key *key1,
const union xfs_btree_key *key2,
const union xfs_btree_key *mask);
diff --git a/libxfs/xfs_ialloc_btree.c b/libxfs/xfs_ialloc_btree.c
index d56876c5be..973ae62d39 100644
--- a/libxfs/xfs_ialloc_btree.c
+++ b/libxfs/xfs_ialloc_btree.c
@@ -273,7 +273,7 @@
cur->bc_rec.i.ir_startino;
}
-STATIC int64_t
+STATIC int
xfs_inobt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
@@ -282,8 +282,8 @@
{
ASSERT(!mask || mask->inobt.ir_startino);
- return (int64_t)be32_to_cpu(k1->inobt.ir_startino) -
- be32_to_cpu(k2->inobt.ir_startino);
+ return cmp_int(be32_to_cpu(k1->inobt.ir_startino),
+ be32_to_cpu(k2->inobt.ir_startino));
}
static xfs_failaddr_t
diff --git a/libxfs/xfs_refcount_btree.c b/libxfs/xfs_refcount_btree.c
index 0924ab7eb7..668c788dca 100644
--- a/libxfs/xfs_refcount_btree.c
+++ b/libxfs/xfs_refcount_btree.c
@@ -187,7 +187,7 @@
return (int64_t)be32_to_cpu(kp->rc_startblock) - start;
}
-STATIC int64_t
+STATIC int
xfs_refcountbt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
@@ -196,8 +196,8 @@
{
ASSERT(!mask || mask->refc.rc_startblock);
- return (int64_t)be32_to_cpu(k1->refc.rc_startblock) -
- be32_to_cpu(k2->refc.rc_startblock);
+ return cmp_int(be32_to_cpu(k1->refc.rc_startblock),
+ be32_to_cpu(k2->refc.rc_startblock));
}
STATIC xfs_failaddr_t
diff --git a/libxfs/xfs_rmap_btree.c b/libxfs/xfs_rmap_btree.c
index ea946616bf..ab207b9cc2 100644
--- a/libxfs/xfs_rmap_btree.c
+++ b/libxfs/xfs_rmap_btree.c
@@ -272,7 +272,7 @@
return 0;
}
-STATIC int64_t
+STATIC int
xfs_rmapbt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
@@ -281,36 +281,31 @@
{
const struct xfs_rmap_key *kp1 = &k1->rmap;
const struct xfs_rmap_key *kp2 = &k2->rmap;
- int64_t d;
- __u64 x, y;
+ int d;
/* Doesn't make sense to mask off the physical space part */
ASSERT(!mask || mask->rmap.rm_startblock);
- d = (int64_t)be32_to_cpu(kp1->rm_startblock) -
- be32_to_cpu(kp2->rm_startblock);
+ d = cmp_int(be32_to_cpu(kp1->rm_startblock),
+ be32_to_cpu(kp2->rm_startblock));
if (d)
return d;
if (!mask || mask->rmap.rm_owner) {
- x = be64_to_cpu(kp1->rm_owner);
- y = be64_to_cpu(kp2->rm_owner);
- if (x > y)
- return 1;
- else if (y > x)
- return -1;
+ d = cmp_int(be64_to_cpu(kp1->rm_owner),
+ be64_to_cpu(kp2->rm_owner));
+ if (d)
+ return d;
}
if (!mask || mask->rmap.rm_offset) {
/* Doesn't make sense to allow offset but not owner */
ASSERT(!mask || mask->rmap.rm_owner);
- x = offset_keymask(be64_to_cpu(kp1->rm_offset));
- y = offset_keymask(be64_to_cpu(kp2->rm_offset));
- if (x > y)
- return 1;
- else if (y > x)
- return -1;
+ d = cmp_int(offset_keymask(be64_to_cpu(kp1->rm_offset)),
+ offset_keymask(be64_to_cpu(kp2->rm_offset)));
+ if (d)
+ return d;
}
return 0;
diff --git a/libxfs/xfs_rtrefcount_btree.c b/libxfs/xfs_rtrefcount_btree.c
index 7a4eec49ca..7fbbc6387c 100644
--- a/libxfs/xfs_rtrefcount_btree.c
+++ b/libxfs/xfs_rtrefcount_btree.c
@@ -168,7 +168,7 @@
return (int64_t)be32_to_cpu(kp->rc_startblock) - start;
}
-STATIC int64_t
+STATIC int
xfs_rtrefcountbt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
@@ -177,8 +177,8 @@
{
ASSERT(!mask || mask->refc.rc_startblock);
- return (int64_t)be32_to_cpu(k1->refc.rc_startblock) -
- be32_to_cpu(k2->refc.rc_startblock);
+ return cmp_int(be32_to_cpu(k1->refc.rc_startblock),
+ be32_to_cpu(k2->refc.rc_startblock));
}
static xfs_failaddr_t
diff --git a/libxfs/xfs_rtrmap_btree.c b/libxfs/xfs_rtrmap_btree.c
index 59a99bb42c..0492cd55d5 100644
--- a/libxfs/xfs_rtrmap_btree.c
+++ b/libxfs/xfs_rtrmap_btree.c
@@ -214,7 +214,7 @@
return 0;
}
-STATIC int64_t
+STATIC int
xfs_rtrmapbt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
@@ -223,36 +223,31 @@
{
const struct xfs_rmap_key *kp1 = &k1->rmap;
const struct xfs_rmap_key *kp2 = &k2->rmap;
- int64_t d;
- __u64 x, y;
+ int d;
/* Doesn't make sense to mask off the physical space part */
ASSERT(!mask || mask->rmap.rm_startblock);
- d = (int64_t)be32_to_cpu(kp1->rm_startblock) -
- be32_to_cpu(kp2->rm_startblock);
+ d = cmp_int(be32_to_cpu(kp1->rm_startblock),
+ be32_to_cpu(kp2->rm_startblock));
if (d)
return d;
if (!mask || mask->rmap.rm_owner) {
- x = be64_to_cpu(kp1->rm_owner);
- y = be64_to_cpu(kp2->rm_owner);
- if (x > y)
- return 1;
- else if (y > x)
- return -1;
+ d = cmp_int(be64_to_cpu(kp1->rm_owner),
+ be64_to_cpu(kp2->rm_owner));
+ if (d)
+ return d;
}
if (!mask || mask->rmap.rm_offset) {
/* Doesn't make sense to allow offset but not owner */
ASSERT(!mask || mask->rmap.rm_owner);
- x = offset_keymask(be64_to_cpu(kp1->rm_offset));
- y = offset_keymask(be64_to_cpu(kp2->rm_offset));
- if (x > y)
- return 1;
- else if (y > x)
- return -1;
+ d = cmp_int(offset_keymask(be64_to_cpu(kp1->rm_offset)),
+ offset_keymask(be64_to_cpu(kp2->rm_offset)));
+ if (d)
+ return d;
}
return 0;
diff --git a/repair/rcbag_btree.c b/repair/rcbag_btree.c
index 704e66e9fb..c42a2ca0d1 100644
--- a/repair/rcbag_btree.c
+++ b/repair/rcbag_btree.c
@@ -72,7 +72,7 @@
return 0;
}
-STATIC int64_t
+STATIC int
rcbagbt_cmp_two_keys(
struct xfs_btree_cur *cur,
const union xfs_btree_key *k1,
diff --git a/scrub/inodes.c b/scrub/inodes.c
index 2f3c87be79..4ed7cd9963 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -197,8 +197,6 @@
return seen_mask;
}
-#define cmp_int(l, r) ((l > r) - (l < r))
-
/* Compare two bulkstat records by inumber. */
static int
compare_bstat(
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 3/11] [PATCH] xfs: refactor cmp_two_keys routines to take advantage of cmp_int()
2025-10-08 16:55 ` [PATCH 3/11] [PATCH] xfs: refactor cmp_two_keys routines to take advantage of cmp_int() Fedor Pchelkin
@ 2025-10-08 20:36 ` Darrick J. Wong
0 siblings, 0 replies; 15+ messages in thread
From: Darrick J. Wong @ 2025-10-08 20:36 UTC (permalink / raw)
To: Fedor Pchelkin
Cc: linux-xfs, aalbersh, cem, cmaiolino, hch, pranav.tyagi03, sandeen
On Wed, Oct 08, 2025 at 06:55:10PM +0200, Fedor Pchelkin wrote:
> Source kernel commit: 3b583adf55c649d5ba37bcd1ca87644b0bc10b86
>
> The net value of these functions is to determine the result of a
> three-way-comparison between operands of the same type.
>
> Simplify the code using cmp_int() to eliminate potential errors with
> opencoded casts and subtractions. This also means we can change the return
> value type of cmp_two_keys routines from int64_t to int and make the
> interface a bit clearer.
>
> Found by Linux Verification Center (linuxtesting.org).
>
> Suggested-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Carlos Maiolino <cem@kernel.org>
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> ---
> include/platform_defs.h | 2 ++
> libxfs/xfs_alloc_btree.c | 21 ++++++++-------------
> libxfs/xfs_bmap_btree.c | 18 +++---------------
> libxfs/xfs_btree.h | 2 +-
> libxfs/xfs_ialloc_btree.c | 6 +++---
> libxfs/xfs_refcount_btree.c | 6 +++---
> libxfs/xfs_rmap_btree.c | 29 ++++++++++++-----------------
> libxfs/xfs_rtrefcount_btree.c | 6 +++---
> libxfs/xfs_rtrmap_btree.c | 29 ++++++++++++-----------------
> repair/rcbag_btree.c | 2 +-
> scrub/inodes.c | 2 --
> 11 files changed, 48 insertions(+), 75 deletions(-)
>
> diff --git a/include/platform_defs.h b/include/platform_defs.h
> index 74a00583eb..fa66551d99 100644
> --- a/include/platform_defs.h
> +++ b/include/platform_defs.h
> @@ -294,4 +294,6 @@
> __a > __b ? (__a - __b) : (__b - __a); \
> })
>
> +#define cmp_int(l, r) ((l > r) - (l < r))
> +
> #endif /* __XFS_PLATFORM_DEFS_H__ */
> diff --git a/libxfs/xfs_alloc_btree.c b/libxfs/xfs_alloc_btree.c
> index 6e7af0020b..c3c69a9de3 100644
> --- a/libxfs/xfs_alloc_btree.c
> +++ b/libxfs/xfs_alloc_btree.c
> @@ -211,7 +211,7 @@
> return (int64_t)be32_to_cpu(kp->ar_startblock) - rec->ar_startblock;
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_bnobt_cmp_two_keys(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *k1,
> @@ -220,29 +220,24 @@
> {
> ASSERT(!mask || mask->alloc.ar_startblock);
>
> - return (int64_t)be32_to_cpu(k1->alloc.ar_startblock) -
> - be32_to_cpu(k2->alloc.ar_startblock);
> + return cmp_int(be32_to_cpu(k1->alloc.ar_startblock),
> + be32_to_cpu(k2->alloc.ar_startblock));
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_cntbt_cmp_two_keys(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *k1,
> const union xfs_btree_key *k2,
> const union xfs_btree_key *mask)
> {
> - int64_t diff;
> -
> ASSERT(!mask || (mask->alloc.ar_blockcount &&
> mask->alloc.ar_startblock));
>
> - diff = be32_to_cpu(k1->alloc.ar_blockcount) -
> - be32_to_cpu(k2->alloc.ar_blockcount);
> - if (diff)
> - return diff;
> -
> - return be32_to_cpu(k1->alloc.ar_startblock) -
> - be32_to_cpu(k2->alloc.ar_startblock);
> + return cmp_int(be32_to_cpu(k1->alloc.ar_blockcount),
> + be32_to_cpu(k2->alloc.ar_blockcount)) ?:
> + cmp_int(be32_to_cpu(k1->alloc.ar_startblock),
> + be32_to_cpu(k2->alloc.ar_startblock));
> }
>
> static xfs_failaddr_t
> diff --git a/libxfs/xfs_bmap_btree.c b/libxfs/xfs_bmap_btree.c
> index 3fc23444f3..19eab66fad 100644
> --- a/libxfs/xfs_bmap_btree.c
> +++ b/libxfs/xfs_bmap_btree.c
> @@ -377,29 +377,17 @@
> cur->bc_rec.b.br_startoff;
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_bmbt_cmp_two_keys(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *k1,
> const union xfs_btree_key *k2,
> const union xfs_btree_key *mask)
> {
> - uint64_t a = be64_to_cpu(k1->bmbt.br_startoff);
> - uint64_t b = be64_to_cpu(k2->bmbt.br_startoff);
> -
> ASSERT(!mask || mask->bmbt.br_startoff);
>
> - /*
> - * Note: This routine previously casted a and b to int64 and subtracted
> - * them to generate a result. This lead to problems if b was the
> - * "maximum" key value (all ones) being signed incorrectly, hence this
> - * somewhat less efficient version.
> - */
> - if (a > b)
> - return 1;
> - if (b > a)
> - return -1;
> - return 0;
> + return cmp_int(be64_to_cpu(k1->bmbt.br_startoff),
> + be64_to_cpu(k2->bmbt.br_startoff));
> }
>
> static xfs_failaddr_t
> diff --git a/libxfs/xfs_btree.h b/libxfs/xfs_btree.h
> index e72a10ba7e..fecd9f0b93 100644
> --- a/libxfs/xfs_btree.h
> +++ b/libxfs/xfs_btree.h
> @@ -184,7 +184,7 @@
> * each key field to be used in the comparison must contain a nonzero
> * value.
> */
> - int64_t (*cmp_two_keys)(struct xfs_btree_cur *cur,
> + int (*cmp_two_keys)(struct xfs_btree_cur *cur,
> const union xfs_btree_key *key1,
> const union xfs_btree_key *key2,
> const union xfs_btree_key *mask);
> diff --git a/libxfs/xfs_ialloc_btree.c b/libxfs/xfs_ialloc_btree.c
> index d56876c5be..973ae62d39 100644
> --- a/libxfs/xfs_ialloc_btree.c
> +++ b/libxfs/xfs_ialloc_btree.c
> @@ -273,7 +273,7 @@
> cur->bc_rec.i.ir_startino;
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_inobt_cmp_two_keys(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *k1,
> @@ -282,8 +282,8 @@
> {
> ASSERT(!mask || mask->inobt.ir_startino);
>
> - return (int64_t)be32_to_cpu(k1->inobt.ir_startino) -
> - be32_to_cpu(k2->inobt.ir_startino);
> + return cmp_int(be32_to_cpu(k1->inobt.ir_startino),
> + be32_to_cpu(k2->inobt.ir_startino));
> }
>
> static xfs_failaddr_t
> diff --git a/libxfs/xfs_refcount_btree.c b/libxfs/xfs_refcount_btree.c
> index 0924ab7eb7..668c788dca 100644
> --- a/libxfs/xfs_refcount_btree.c
> +++ b/libxfs/xfs_refcount_btree.c
> @@ -187,7 +187,7 @@
> return (int64_t)be32_to_cpu(kp->rc_startblock) - start;
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_refcountbt_cmp_two_keys(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *k1,
> @@ -196,8 +196,8 @@
> {
> ASSERT(!mask || mask->refc.rc_startblock);
>
> - return (int64_t)be32_to_cpu(k1->refc.rc_startblock) -
> - be32_to_cpu(k2->refc.rc_startblock);
> + return cmp_int(be32_to_cpu(k1->refc.rc_startblock),
> + be32_to_cpu(k2->refc.rc_startblock));
> }
>
> STATIC xfs_failaddr_t
> diff --git a/libxfs/xfs_rmap_btree.c b/libxfs/xfs_rmap_btree.c
> index ea946616bf..ab207b9cc2 100644
> --- a/libxfs/xfs_rmap_btree.c
> +++ b/libxfs/xfs_rmap_btree.c
> @@ -272,7 +272,7 @@
> return 0;
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_rmapbt_cmp_two_keys(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *k1,
> @@ -281,36 +281,31 @@
> {
> const struct xfs_rmap_key *kp1 = &k1->rmap;
> const struct xfs_rmap_key *kp2 = &k2->rmap;
> - int64_t d;
> - __u64 x, y;
> + int d;
>
> /* Doesn't make sense to mask off the physical space part */
> ASSERT(!mask || mask->rmap.rm_startblock);
>
> - d = (int64_t)be32_to_cpu(kp1->rm_startblock) -
> - be32_to_cpu(kp2->rm_startblock);
> + d = cmp_int(be32_to_cpu(kp1->rm_startblock),
> + be32_to_cpu(kp2->rm_startblock));
> if (d)
> return d;
>
> if (!mask || mask->rmap.rm_owner) {
> - x = be64_to_cpu(kp1->rm_owner);
> - y = be64_to_cpu(kp2->rm_owner);
> - if (x > y)
> - return 1;
> - else if (y > x)
> - return -1;
> + d = cmp_int(be64_to_cpu(kp1->rm_owner),
> + be64_to_cpu(kp2->rm_owner));
> + if (d)
> + return d;
> }
>
> if (!mask || mask->rmap.rm_offset) {
> /* Doesn't make sense to allow offset but not owner */
> ASSERT(!mask || mask->rmap.rm_owner);
>
> - x = offset_keymask(be64_to_cpu(kp1->rm_offset));
> - y = offset_keymask(be64_to_cpu(kp2->rm_offset));
> - if (x > y)
> - return 1;
> - else if (y > x)
> - return -1;
> + d = cmp_int(offset_keymask(be64_to_cpu(kp1->rm_offset)),
> + offset_keymask(be64_to_cpu(kp2->rm_offset)));
> + if (d)
> + return d;
> }
>
> return 0;
> diff --git a/libxfs/xfs_rtrefcount_btree.c b/libxfs/xfs_rtrefcount_btree.c
> index 7a4eec49ca..7fbbc6387c 100644
> --- a/libxfs/xfs_rtrefcount_btree.c
> +++ b/libxfs/xfs_rtrefcount_btree.c
> @@ -168,7 +168,7 @@
> return (int64_t)be32_to_cpu(kp->rc_startblock) - start;
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_rtrefcountbt_cmp_two_keys(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *k1,
> @@ -177,8 +177,8 @@
> {
> ASSERT(!mask || mask->refc.rc_startblock);
>
> - return (int64_t)be32_to_cpu(k1->refc.rc_startblock) -
> - be32_to_cpu(k2->refc.rc_startblock);
> + return cmp_int(be32_to_cpu(k1->refc.rc_startblock),
> + be32_to_cpu(k2->refc.rc_startblock));
> }
>
> static xfs_failaddr_t
> diff --git a/libxfs/xfs_rtrmap_btree.c b/libxfs/xfs_rtrmap_btree.c
> index 59a99bb42c..0492cd55d5 100644
> --- a/libxfs/xfs_rtrmap_btree.c
> +++ b/libxfs/xfs_rtrmap_btree.c
> @@ -214,7 +214,7 @@
> return 0;
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_rtrmapbt_cmp_two_keys(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *k1,
> @@ -223,36 +223,31 @@
> {
> const struct xfs_rmap_key *kp1 = &k1->rmap;
> const struct xfs_rmap_key *kp2 = &k2->rmap;
> - int64_t d;
> - __u64 x, y;
> + int d;
>
> /* Doesn't make sense to mask off the physical space part */
> ASSERT(!mask || mask->rmap.rm_startblock);
>
> - d = (int64_t)be32_to_cpu(kp1->rm_startblock) -
> - be32_to_cpu(kp2->rm_startblock);
> + d = cmp_int(be32_to_cpu(kp1->rm_startblock),
> + be32_to_cpu(kp2->rm_startblock));
> if (d)
> return d;
>
> if (!mask || mask->rmap.rm_owner) {
> - x = be64_to_cpu(kp1->rm_owner);
> - y = be64_to_cpu(kp2->rm_owner);
> - if (x > y)
> - return 1;
> - else if (y > x)
> - return -1;
> + d = cmp_int(be64_to_cpu(kp1->rm_owner),
> + be64_to_cpu(kp2->rm_owner));
> + if (d)
> + return d;
> }
>
> if (!mask || mask->rmap.rm_offset) {
> /* Doesn't make sense to allow offset but not owner */
> ASSERT(!mask || mask->rmap.rm_owner);
>
> - x = offset_keymask(be64_to_cpu(kp1->rm_offset));
> - y = offset_keymask(be64_to_cpu(kp2->rm_offset));
> - if (x > y)
> - return 1;
> - else if (y > x)
> - return -1;
> + d = cmp_int(offset_keymask(be64_to_cpu(kp1->rm_offset)),
> + offset_keymask(be64_to_cpu(kp2->rm_offset)));
> + if (d)
> + return d;
> }
>
> return 0;
> diff --git a/repair/rcbag_btree.c b/repair/rcbag_btree.c
> index 704e66e9fb..c42a2ca0d1 100644
> --- a/repair/rcbag_btree.c
> +++ b/repair/rcbag_btree.c
> @@ -72,7 +72,7 @@
> return 0;
> }
>
> -STATIC int64_t
> +STATIC int
> rcbagbt_cmp_two_keys(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *k1,
Dumb nit: You could port this one too:
@@ -81,25 +81,19 @@ rcbagbt_cmp_two_keys(
{
const struct rcbag_key *kp1 = (const struct rcbag_key *)k1;
const struct rcbag_key *kp2 = (const struct rcbag_key *)k2;
+ int d;
ASSERT(mask == NULL);
- if (kp1->rbg_startblock > kp2->rbg_startblock)
- return 1;
- if (kp1->rbg_startblock < kp2->rbg_startblock)
- return -1;
-
- if (kp1->rbg_blockcount > kp2->rbg_blockcount)
- return 1;
- if (kp1->rbg_blockcount < kp2->rbg_blockcount)
- return -1;
+ d = cmp_int(kp1->rbg_startblock, kp2->rbg_startblock);
+ if (d)
+ return d;
- if (kp1->rbg_ino > kp2->rbg_ino)
- return 1;
- if (kp1->rbg_ino < kp2->rbg_ino)
- return -1;
+ d = cmp_int(kp1->rbg_blockcount, kp2->rbg_blockcount);
+ if (d)
+ return d;
- return 0;
+ return cmp_int(kp1->rbg_ino, kp2->rbg_ino);
}
STATIC int
--D
> diff --git a/scrub/inodes.c b/scrub/inodes.c
> index 2f3c87be79..4ed7cd9963 100644
> --- a/scrub/inodes.c
> +++ b/scrub/inodes.c
> @@ -197,8 +197,6 @@
> return seen_mask;
> }
>
> -#define cmp_int(l, r) ((l > r) - (l < r))
> -
> /* Compare two bulkstat records by inumber. */
> static int
> compare_bstat(
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/11] [PATCH] xfs: refactor cmp_key_with_cur routines to take advantage of cmp_int()
2025-10-08 16:41 [PATCH 0/12] xfsprogs: libxfs sync v6.17 Andrey Albershteyn
` (2 preceding siblings ...)
2025-10-08 16:55 ` [PATCH 3/11] [PATCH] xfs: refactor cmp_two_keys routines to take advantage of cmp_int() Fedor Pchelkin
@ 2025-10-08 16:55 ` Fedor Pchelkin
2025-10-08 20:36 ` Darrick J. Wong
2025-10-08 16:55 ` [PATCH 5/11] [PATCH] xfs: use a proper variable name and type for storing a comparison result Fedor Pchelkin
` (7 subsequent siblings)
11 siblings, 1 reply; 15+ messages in thread
From: Fedor Pchelkin @ 2025-10-08 16:55 UTC (permalink / raw)
To: linux-xfs, aalbersh, cem, cmaiolino, djwong, hch, pchelkin,
pranav.tyagi03, sandeen
Source kernel commit: 734b871d6cf7d4f815bb1eff8c808289079701c2
The net value of these functions is to determine the result of a
three-way-comparison between operands of the same type.
Simplify the code using cmp_int() to eliminate potential errors with
opencoded casts and subtractions. This also means we can change the return
value type of cmp_key_with_cur routines from int64_t to int and make the
interface a bit clearer.
Found by Linux Verification Center (linuxtesting.org).
Suggested-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
libxfs/xfs_alloc_btree.c | 17 +++++++----------
libxfs/xfs_bmap_btree.c | 6 +++---
libxfs/xfs_btree.h | 2 +-
libxfs/xfs_ialloc_btree.c | 6 +++---
libxfs/xfs_refcount_btree.c | 4 ++--
libxfs/xfs_rmap_btree.c | 28 ++++++----------------------
libxfs/xfs_rtrefcount_btree.c | 4 ++--
libxfs/xfs_rtrmap_btree.c | 28 ++++++----------------------
repair/rcbag_btree.c | 2 +-
9 files changed, 31 insertions(+), 66 deletions(-)
diff --git a/libxfs/xfs_alloc_btree.c b/libxfs/xfs_alloc_btree.c
index c3c69a9de3..1604e1bb62 100644
--- a/libxfs/xfs_alloc_btree.c
+++ b/libxfs/xfs_alloc_btree.c
@@ -184,7 +184,7 @@
ptr->s = agf->agf_cnt_root;
}
-STATIC int64_t
+STATIC int
xfs_bnobt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
@@ -192,23 +192,20 @@
struct xfs_alloc_rec_incore *rec = &cur->bc_rec.a;
const struct xfs_alloc_rec *kp = &key->alloc;
- return (int64_t)be32_to_cpu(kp->ar_startblock) - rec->ar_startblock;
+ return cmp_int(be32_to_cpu(kp->ar_startblock),
+ rec->ar_startblock);
}
-STATIC int64_t
+STATIC int
xfs_cntbt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
{
struct xfs_alloc_rec_incore *rec = &cur->bc_rec.a;
const struct xfs_alloc_rec *kp = &key->alloc;
- int64_t diff;
-
- diff = (int64_t)be32_to_cpu(kp->ar_blockcount) - rec->ar_blockcount;
- if (diff)
- return diff;
-
- return (int64_t)be32_to_cpu(kp->ar_startblock) - rec->ar_startblock;
+
+ return cmp_int(be32_to_cpu(kp->ar_blockcount), rec->ar_blockcount) ?:
+ cmp_int(be32_to_cpu(kp->ar_startblock), rec->ar_startblock);
}
STATIC int
diff --git a/libxfs/xfs_bmap_btree.c b/libxfs/xfs_bmap_btree.c
index 19eab66fad..252da347b0 100644
--- a/libxfs/xfs_bmap_btree.c
+++ b/libxfs/xfs_bmap_btree.c
@@ -368,13 +368,13 @@
xfs_bmbt_disk_set_all(&rec->bmbt, &cur->bc_rec.b);
}
-STATIC int64_t
+STATIC int
xfs_bmbt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
{
- return (int64_t)be64_to_cpu(key->bmbt.br_startoff) -
- cur->bc_rec.b.br_startoff;
+ return cmp_int(be64_to_cpu(key->bmbt.br_startoff),
+ cur->bc_rec.b.br_startoff);
}
STATIC int
diff --git a/libxfs/xfs_btree.h b/libxfs/xfs_btree.h
index fecd9f0b93..1bf20d509a 100644
--- a/libxfs/xfs_btree.h
+++ b/libxfs/xfs_btree.h
@@ -175,7 +175,7 @@
* Compare key value and cursor value -- positive if key > cur,
* negative if key < cur, and zero if equal.
*/
- int64_t (*cmp_key_with_cur)(struct xfs_btree_cur *cur,
+ int (*cmp_key_with_cur)(struct xfs_btree_cur *cur,
const union xfs_btree_key *key);
/*
diff --git a/libxfs/xfs_ialloc_btree.c b/libxfs/xfs_ialloc_btree.c
index 973ae62d39..dab9b61bd2 100644
--- a/libxfs/xfs_ialloc_btree.c
+++ b/libxfs/xfs_ialloc_btree.c
@@ -264,13 +264,13 @@
ptr->s = agi->agi_free_root;
}
-STATIC int64_t
+STATIC int
xfs_inobt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
{
- return (int64_t)be32_to_cpu(key->inobt.ir_startino) -
- cur->bc_rec.i.ir_startino;
+ return cmp_int(be32_to_cpu(key->inobt.ir_startino),
+ cur->bc_rec.i.ir_startino);
}
STATIC int
diff --git a/libxfs/xfs_refcount_btree.c b/libxfs/xfs_refcount_btree.c
index 668c788dca..44d942e9d0 100644
--- a/libxfs/xfs_refcount_btree.c
+++ b/libxfs/xfs_refcount_btree.c
@@ -173,7 +173,7 @@
ptr->s = agf->agf_refcount_root;
}
-STATIC int64_t
+STATIC int
xfs_refcountbt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
@@ -184,7 +184,7 @@
start = xfs_refcount_encode_startblock(irec->rc_startblock,
irec->rc_domain);
- return (int64_t)be32_to_cpu(kp->rc_startblock) - start;
+ return cmp_int(be32_to_cpu(kp->rc_startblock), start);
}
STATIC int
diff --git a/libxfs/xfs_rmap_btree.c b/libxfs/xfs_rmap_btree.c
index ab207b9cc2..d7b9fccc3a 100644
--- a/libxfs/xfs_rmap_btree.c
+++ b/libxfs/xfs_rmap_btree.c
@@ -242,34 +242,18 @@
return offset & ~XFS_RMAP_OFF_UNWRITTEN;
}
-STATIC int64_t
+STATIC int
xfs_rmapbt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
{
struct xfs_rmap_irec *rec = &cur->bc_rec.r;
const struct xfs_rmap_key *kp = &key->rmap;
- __u64 x, y;
- int64_t d;
-
- d = (int64_t)be32_to_cpu(kp->rm_startblock) - rec->rm_startblock;
- if (d)
- return d;
-
- x = be64_to_cpu(kp->rm_owner);
- y = rec->rm_owner;
- if (x > y)
- return 1;
- else if (y > x)
- return -1;
-
- x = offset_keymask(be64_to_cpu(kp->rm_offset));
- y = offset_keymask(xfs_rmap_irec_offset_pack(rec));
- if (x > y)
- return 1;
- else if (y > x)
- return -1;
- return 0;
+
+ return cmp_int(be32_to_cpu(kp->rm_startblock), rec->rm_startblock) ?:
+ cmp_int(be64_to_cpu(kp->rm_owner), rec->rm_owner) ?:
+ cmp_int(offset_keymask(be64_to_cpu(kp->rm_offset)),
+ offset_keymask(xfs_rmap_irec_offset_pack(rec)));
}
STATIC int
diff --git a/libxfs/xfs_rtrefcount_btree.c b/libxfs/xfs_rtrefcount_btree.c
index 7fbbc6387c..77191f073a 100644
--- a/libxfs/xfs_rtrefcount_btree.c
+++ b/libxfs/xfs_rtrefcount_btree.c
@@ -154,7 +154,7 @@
ptr->l = 0;
}
-STATIC int64_t
+STATIC int
xfs_rtrefcountbt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
@@ -165,7 +165,7 @@
start = xfs_refcount_encode_startblock(irec->rc_startblock,
irec->rc_domain);
- return (int64_t)be32_to_cpu(kp->rc_startblock) - start;
+ return cmp_int(be32_to_cpu(kp->rc_startblock), start);
}
STATIC int
diff --git a/libxfs/xfs_rtrmap_btree.c b/libxfs/xfs_rtrmap_btree.c
index 0492cd55d5..633dca0333 100644
--- a/libxfs/xfs_rtrmap_btree.c
+++ b/libxfs/xfs_rtrmap_btree.c
@@ -184,34 +184,18 @@
return offset & ~XFS_RMAP_OFF_UNWRITTEN;
}
-STATIC int64_t
+STATIC int
xfs_rtrmapbt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
{
struct xfs_rmap_irec *rec = &cur->bc_rec.r;
const struct xfs_rmap_key *kp = &key->rmap;
- __u64 x, y;
- int64_t d;
-
- d = (int64_t)be32_to_cpu(kp->rm_startblock) - rec->rm_startblock;
- if (d)
- return d;
-
- x = be64_to_cpu(kp->rm_owner);
- y = rec->rm_owner;
- if (x > y)
- return 1;
- else if (y > x)
- return -1;
-
- x = offset_keymask(be64_to_cpu(kp->rm_offset));
- y = offset_keymask(xfs_rmap_irec_offset_pack(rec));
- if (x > y)
- return 1;
- else if (y > x)
- return -1;
- return 0;
+
+ return cmp_int(be32_to_cpu(kp->rm_startblock), rec->rm_startblock) ?:
+ cmp_int(be64_to_cpu(kp->rm_owner), rec->rm_owner) ?:
+ cmp_int(offset_keymask(be64_to_cpu(kp->rm_offset)),
+ offset_keymask(xfs_rmap_irec_offset_pack(rec)));
}
STATIC int
diff --git a/repair/rcbag_btree.c b/repair/rcbag_btree.c
index c42a2ca0d1..fc5f69c4d2 100644
--- a/repair/rcbag_btree.c
+++ b/repair/rcbag_btree.c
@@ -46,7 +46,7 @@
bag_rec->rbg_refcount = bag_irec->rbg_refcount;
}
-STATIC int64_t
+STATIC int
rcbagbt_cmp_key_with_cur(
struct xfs_btree_cur *cur,
const union xfs_btree_key *key)
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 4/11] [PATCH] xfs: refactor cmp_key_with_cur routines to take advantage of cmp_int()
2025-10-08 16:55 ` [PATCH 4/11] [PATCH] xfs: refactor cmp_key_with_cur " Fedor Pchelkin
@ 2025-10-08 20:36 ` Darrick J. Wong
0 siblings, 0 replies; 15+ messages in thread
From: Darrick J. Wong @ 2025-10-08 20:36 UTC (permalink / raw)
To: Fedor Pchelkin
Cc: linux-xfs, aalbersh, cem, cmaiolino, hch, pranav.tyagi03, sandeen
On Wed, Oct 08, 2025 at 06:55:24PM +0200, Fedor Pchelkin wrote:
> Source kernel commit: 734b871d6cf7d4f815bb1eff8c808289079701c2
>
> The net value of these functions is to determine the result of a
> three-way-comparison between operands of the same type.
>
> Simplify the code using cmp_int() to eliminate potential errors with
> opencoded casts and subtractions. This also means we can change the return
> value type of cmp_key_with_cur routines from int64_t to int and make the
> interface a bit clearer.
>
> Found by Linux Verification Center (linuxtesting.org).
>
> Suggested-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Carlos Maiolino <cem@kernel.org>
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> ---
> libxfs/xfs_alloc_btree.c | 17 +++++++----------
> libxfs/xfs_bmap_btree.c | 6 +++---
> libxfs/xfs_btree.h | 2 +-
> libxfs/xfs_ialloc_btree.c | 6 +++---
> libxfs/xfs_refcount_btree.c | 4 ++--
> libxfs/xfs_rmap_btree.c | 28 ++++++----------------------
> libxfs/xfs_rtrefcount_btree.c | 4 ++--
> libxfs/xfs_rtrmap_btree.c | 28 ++++++----------------------
> repair/rcbag_btree.c | 2 +-
> 9 files changed, 31 insertions(+), 66 deletions(-)
>
> diff --git a/libxfs/xfs_alloc_btree.c b/libxfs/xfs_alloc_btree.c
> index c3c69a9de3..1604e1bb62 100644
> --- a/libxfs/xfs_alloc_btree.c
> +++ b/libxfs/xfs_alloc_btree.c
> @@ -184,7 +184,7 @@
> ptr->s = agf->agf_cnt_root;
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_bnobt_cmp_key_with_cur(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *key)
> @@ -192,23 +192,20 @@
> struct xfs_alloc_rec_incore *rec = &cur->bc_rec.a;
> const struct xfs_alloc_rec *kp = &key->alloc;
>
> - return (int64_t)be32_to_cpu(kp->ar_startblock) - rec->ar_startblock;
> + return cmp_int(be32_to_cpu(kp->ar_startblock),
> + rec->ar_startblock);
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_cntbt_cmp_key_with_cur(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *key)
> {
> struct xfs_alloc_rec_incore *rec = &cur->bc_rec.a;
> const struct xfs_alloc_rec *kp = &key->alloc;
> - int64_t diff;
> -
> - diff = (int64_t)be32_to_cpu(kp->ar_blockcount) - rec->ar_blockcount;
> - if (diff)
> - return diff;
> -
> - return (int64_t)be32_to_cpu(kp->ar_startblock) - rec->ar_startblock;
> +
> + return cmp_int(be32_to_cpu(kp->ar_blockcount), rec->ar_blockcount) ?:
> + cmp_int(be32_to_cpu(kp->ar_startblock), rec->ar_startblock);
> }
>
> STATIC int
> diff --git a/libxfs/xfs_bmap_btree.c b/libxfs/xfs_bmap_btree.c
> index 19eab66fad..252da347b0 100644
> --- a/libxfs/xfs_bmap_btree.c
> +++ b/libxfs/xfs_bmap_btree.c
> @@ -368,13 +368,13 @@
> xfs_bmbt_disk_set_all(&rec->bmbt, &cur->bc_rec.b);
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_bmbt_cmp_key_with_cur(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *key)
> {
> - return (int64_t)be64_to_cpu(key->bmbt.br_startoff) -
> - cur->bc_rec.b.br_startoff;
> + return cmp_int(be64_to_cpu(key->bmbt.br_startoff),
> + cur->bc_rec.b.br_startoff);
> }
>
> STATIC int
> diff --git a/libxfs/xfs_btree.h b/libxfs/xfs_btree.h
> index fecd9f0b93..1bf20d509a 100644
> --- a/libxfs/xfs_btree.h
> +++ b/libxfs/xfs_btree.h
> @@ -175,7 +175,7 @@
> * Compare key value and cursor value -- positive if key > cur,
> * negative if key < cur, and zero if equal.
> */
> - int64_t (*cmp_key_with_cur)(struct xfs_btree_cur *cur,
> + int (*cmp_key_with_cur)(struct xfs_btree_cur *cur,
> const union xfs_btree_key *key);
>
> /*
> diff --git a/libxfs/xfs_ialloc_btree.c b/libxfs/xfs_ialloc_btree.c
> index 973ae62d39..dab9b61bd2 100644
> --- a/libxfs/xfs_ialloc_btree.c
> +++ b/libxfs/xfs_ialloc_btree.c
> @@ -264,13 +264,13 @@
> ptr->s = agi->agi_free_root;
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_inobt_cmp_key_with_cur(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *key)
> {
> - return (int64_t)be32_to_cpu(key->inobt.ir_startino) -
> - cur->bc_rec.i.ir_startino;
> + return cmp_int(be32_to_cpu(key->inobt.ir_startino),
> + cur->bc_rec.i.ir_startino);
> }
>
> STATIC int
> diff --git a/libxfs/xfs_refcount_btree.c b/libxfs/xfs_refcount_btree.c
> index 668c788dca..44d942e9d0 100644
> --- a/libxfs/xfs_refcount_btree.c
> +++ b/libxfs/xfs_refcount_btree.c
> @@ -173,7 +173,7 @@
> ptr->s = agf->agf_refcount_root;
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_refcountbt_cmp_key_with_cur(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *key)
> @@ -184,7 +184,7 @@
>
> start = xfs_refcount_encode_startblock(irec->rc_startblock,
> irec->rc_domain);
> - return (int64_t)be32_to_cpu(kp->rc_startblock) - start;
> + return cmp_int(be32_to_cpu(kp->rc_startblock), start);
> }
>
> STATIC int
> diff --git a/libxfs/xfs_rmap_btree.c b/libxfs/xfs_rmap_btree.c
> index ab207b9cc2..d7b9fccc3a 100644
> --- a/libxfs/xfs_rmap_btree.c
> +++ b/libxfs/xfs_rmap_btree.c
> @@ -242,34 +242,18 @@
> return offset & ~XFS_RMAP_OFF_UNWRITTEN;
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_rmapbt_cmp_key_with_cur(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *key)
> {
> struct xfs_rmap_irec *rec = &cur->bc_rec.r;
> const struct xfs_rmap_key *kp = &key->rmap;
> - __u64 x, y;
> - int64_t d;
> -
> - d = (int64_t)be32_to_cpu(kp->rm_startblock) - rec->rm_startblock;
> - if (d)
> - return d;
> -
> - x = be64_to_cpu(kp->rm_owner);
> - y = rec->rm_owner;
> - if (x > y)
> - return 1;
> - else if (y > x)
> - return -1;
> -
> - x = offset_keymask(be64_to_cpu(kp->rm_offset));
> - y = offset_keymask(xfs_rmap_irec_offset_pack(rec));
> - if (x > y)
> - return 1;
> - else if (y > x)
> - return -1;
> - return 0;
> +
> + return cmp_int(be32_to_cpu(kp->rm_startblock), rec->rm_startblock) ?:
> + cmp_int(be64_to_cpu(kp->rm_owner), rec->rm_owner) ?:
> + cmp_int(offset_keymask(be64_to_cpu(kp->rm_offset)),
> + offset_keymask(xfs_rmap_irec_offset_pack(rec)));
> }
>
> STATIC int
> diff --git a/libxfs/xfs_rtrefcount_btree.c b/libxfs/xfs_rtrefcount_btree.c
> index 7fbbc6387c..77191f073a 100644
> --- a/libxfs/xfs_rtrefcount_btree.c
> +++ b/libxfs/xfs_rtrefcount_btree.c
> @@ -154,7 +154,7 @@
> ptr->l = 0;
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_rtrefcountbt_cmp_key_with_cur(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *key)
> @@ -165,7 +165,7 @@
>
> start = xfs_refcount_encode_startblock(irec->rc_startblock,
> irec->rc_domain);
> - return (int64_t)be32_to_cpu(kp->rc_startblock) - start;
> + return cmp_int(be32_to_cpu(kp->rc_startblock), start);
> }
>
> STATIC int
> diff --git a/libxfs/xfs_rtrmap_btree.c b/libxfs/xfs_rtrmap_btree.c
> index 0492cd55d5..633dca0333 100644
> --- a/libxfs/xfs_rtrmap_btree.c
> +++ b/libxfs/xfs_rtrmap_btree.c
> @@ -184,34 +184,18 @@
> return offset & ~XFS_RMAP_OFF_UNWRITTEN;
> }
>
> -STATIC int64_t
> +STATIC int
> xfs_rtrmapbt_cmp_key_with_cur(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *key)
> {
> struct xfs_rmap_irec *rec = &cur->bc_rec.r;
> const struct xfs_rmap_key *kp = &key->rmap;
> - __u64 x, y;
> - int64_t d;
> -
> - d = (int64_t)be32_to_cpu(kp->rm_startblock) - rec->rm_startblock;
> - if (d)
> - return d;
> -
> - x = be64_to_cpu(kp->rm_owner);
> - y = rec->rm_owner;
> - if (x > y)
> - return 1;
> - else if (y > x)
> - return -1;
> -
> - x = offset_keymask(be64_to_cpu(kp->rm_offset));
> - y = offset_keymask(xfs_rmap_irec_offset_pack(rec));
> - if (x > y)
> - return 1;
> - else if (y > x)
> - return -1;
> - return 0;
> +
> + return cmp_int(be32_to_cpu(kp->rm_startblock), rec->rm_startblock) ?:
> + cmp_int(be64_to_cpu(kp->rm_owner), rec->rm_owner) ?:
> + cmp_int(offset_keymask(be64_to_cpu(kp->rm_offset)),
> + offset_keymask(xfs_rmap_irec_offset_pack(rec)));
> }
>
> STATIC int
> diff --git a/repair/rcbag_btree.c b/repair/rcbag_btree.c
> index c42a2ca0d1..fc5f69c4d2 100644
> --- a/repair/rcbag_btree.c
> +++ b/repair/rcbag_btree.c
> @@ -46,7 +46,7 @@
> bag_rec->rbg_refcount = bag_irec->rbg_refcount;
> }
>
> -STATIC int64_t
> +STATIC int
> rcbagbt_cmp_key_with_cur(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *key)
Same here, this could be ported to cmp_int.
--D
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/11] [PATCH] xfs: use a proper variable name and type for storing a comparison result
2025-10-08 16:41 [PATCH 0/12] xfsprogs: libxfs sync v6.17 Andrey Albershteyn
` (3 preceding siblings ...)
2025-10-08 16:55 ` [PATCH 4/11] [PATCH] xfs: refactor cmp_key_with_cur " Fedor Pchelkin
@ 2025-10-08 16:55 ` Fedor Pchelkin
2025-10-08 16:55 ` [PATCH 6/11] [PATCH] xfs: refactor xfs_btree_diff_two_ptrs() to take advantage of cmp_int() Fedor Pchelkin
` (6 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Fedor Pchelkin @ 2025-10-08 16:55 UTC (permalink / raw)
To: linux-xfs, aalbersh, cem, cmaiolino, djwong, hch, pchelkin,
pranav.tyagi03, sandeen
Source kernel commit: 2717eb35185581988799bb0d5179409978f36a90
Perhaps that's just my silly imagination but 'diff' doesn't look good for
the name of a variable to hold a result of a three-way-comparison
(-1, 0, 1) which is what ->cmp_key_with_cur() does. It implies to contain
an actual difference between the two integer variables but that's not true
anymore after recent refactoring.
Declaring it as int64_t is also misleading now. Plain integer type is
more than enough.
Found by Linux Verification Center (linuxtesting.org).
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
libxfs/xfs_btree.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/libxfs/xfs_btree.c b/libxfs/xfs_btree.c
index 15846f0ff6..facc35401f 100644
--- a/libxfs/xfs_btree.c
+++ b/libxfs/xfs_btree.c
@@ -1982,7 +1982,7 @@
int *stat) /* success/failure */
{
struct xfs_btree_block *block; /* current btree block */
- int64_t diff; /* difference for the current key */
+ int cmp_r; /* current key comparison result */
int error; /* error return value */
int keyno; /* current key number */
int level; /* level in the btree */
@@ -2010,13 +2010,13 @@
* on the lookup record, then follow the corresponding block
* pointer down to the next level.
*/
- for (level = cur->bc_nlevels - 1, diff = 1; level >= 0; level--) {
+ for (level = cur->bc_nlevels - 1, cmp_r = 1; level >= 0; level--) {
/* Get the block we need to do the lookup on. */
error = xfs_btree_lookup_get_block(cur, level, pp, &block);
if (error)
goto error0;
- if (diff == 0) {
+ if (cmp_r == 0) {
/*
* If we already had a key match at a higher level, we
* know we need to use the first entry in this block.
@@ -2062,15 +2062,16 @@
keyno, block, &key);
/*
- * Compute difference to get next direction:
+ * Compute comparison result to get next
+ * direction:
* - less than, move right
* - greater than, move left
* - equal, we're done
*/
- diff = cur->bc_ops->cmp_key_with_cur(cur, kp);
- if (diff < 0)
+ cmp_r = cur->bc_ops->cmp_key_with_cur(cur, kp);
+ if (cmp_r < 0)
low = keyno + 1;
- else if (diff > 0)
+ else if (cmp_r > 0)
high = keyno - 1;
else
break;
@@ -2086,7 +2087,7 @@
* If we moved left, need the previous key number,
* unless there isn't one.
*/
- if (diff > 0 && --keyno < 1)
+ if (cmp_r > 0 && --keyno < 1)
keyno = 1;
pp = xfs_btree_ptr_addr(cur, keyno, block);
@@ -2099,7 +2100,7 @@
}
/* Done with the search. See if we need to adjust the results. */
- if (dir != XFS_LOOKUP_LE && diff < 0) {
+ if (dir != XFS_LOOKUP_LE && cmp_r < 0) {
keyno++;
/*
* If ge search and we went off the end of the block, but it's
@@ -2122,14 +2123,14 @@
*stat = 1;
return 0;
}
- } else if (dir == XFS_LOOKUP_LE && diff > 0)
+ } else if (dir == XFS_LOOKUP_LE && cmp_r > 0)
keyno--;
cur->bc_levels[0].ptr = keyno;
/* Return if we succeeded or not. */
if (keyno == 0 || keyno > xfs_btree_get_numrecs(block))
*stat = 0;
- else if (dir != XFS_LOOKUP_EQ || diff == 0)
+ else if (dir != XFS_LOOKUP_EQ || cmp_r == 0)
*stat = 1;
else
*stat = 0;
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 6/11] [PATCH] xfs: refactor xfs_btree_diff_two_ptrs() to take advantage of cmp_int()
2025-10-08 16:41 [PATCH 0/12] xfsprogs: libxfs sync v6.17 Andrey Albershteyn
` (4 preceding siblings ...)
2025-10-08 16:55 ` [PATCH 5/11] [PATCH] xfs: use a proper variable name and type for storing a comparison result Fedor Pchelkin
@ 2025-10-08 16:55 ` Fedor Pchelkin
2025-10-08 16:56 ` [PATCH 7/11] [PATCH] xfs: return the allocated transaction from xfs_trans_alloc_empty Christoph Hellwig
` (5 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Fedor Pchelkin @ 2025-10-08 16:55 UTC (permalink / raw)
To: linux-xfs, aalbersh, cem, cmaiolino, djwong, hch, pchelkin,
pranav.tyagi03, sandeen
Source kernel commit: ce6cce46aff79423f47680ee65e8f12191a50605
Use cmp_int() to yield the result of a three-way-comparison instead of
performing subtractions with extra casts. Thus also rename the function
to make its name clearer in purpose.
Found by Linux Verification Center (linuxtesting.org).
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
libxfs/xfs_btree.c | 8 ++++----
libxfs/xfs_btree.h | 6 +++---
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/libxfs/xfs_btree.c b/libxfs/xfs_btree.c
index facc35401f..8576611994 100644
--- a/libxfs/xfs_btree.c
+++ b/libxfs/xfs_btree.c
@@ -5350,15 +5350,15 @@
}
/* Compare two btree pointers. */
-int64_t
-xfs_btree_diff_two_ptrs(
+int
+xfs_btree_cmp_two_ptrs(
struct xfs_btree_cur *cur,
const union xfs_btree_ptr *a,
const union xfs_btree_ptr *b)
{
if (cur->bc_ops->ptr_len == XFS_BTREE_LONG_PTR_LEN)
- return (int64_t)be64_to_cpu(a->l) - be64_to_cpu(b->l);
- return (int64_t)be32_to_cpu(a->s) - be32_to_cpu(b->s);
+ return cmp_int(be64_to_cpu(a->l), be64_to_cpu(b->l));
+ return cmp_int(be32_to_cpu(a->s), be32_to_cpu(b->s));
}
struct xfs_btree_has_records {
diff --git a/libxfs/xfs_btree.h b/libxfs/xfs_btree.h
index 1bf20d509a..60e78572e7 100644
--- a/libxfs/xfs_btree.h
+++ b/libxfs/xfs_btree.h
@@ -519,9 +519,9 @@
int level, struct xfs_buf **bpp);
bool xfs_btree_ptr_is_null(struct xfs_btree_cur *cur,
const union xfs_btree_ptr *ptr);
-int64_t xfs_btree_diff_two_ptrs(struct xfs_btree_cur *cur,
- const union xfs_btree_ptr *a,
- const union xfs_btree_ptr *b);
+int xfs_btree_cmp_two_ptrs(struct xfs_btree_cur *cur,
+ const union xfs_btree_ptr *a,
+ const union xfs_btree_ptr *b);
void xfs_btree_get_sibling(struct xfs_btree_cur *cur,
struct xfs_btree_block *block,
union xfs_btree_ptr *ptr, int lr);
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 7/11] [PATCH] xfs: return the allocated transaction from xfs_trans_alloc_empty
2025-10-08 16:41 [PATCH 0/12] xfsprogs: libxfs sync v6.17 Andrey Albershteyn
` (5 preceding siblings ...)
2025-10-08 16:55 ` [PATCH 6/11] [PATCH] xfs: refactor xfs_btree_diff_two_ptrs() to take advantage of cmp_int() Fedor Pchelkin
@ 2025-10-08 16:56 ` Christoph Hellwig
2025-10-08 16:56 ` [PATCH 8/11] [PATCH] xfs: improve the xg_active_ref check in xfs_group_free Christoph Hellwig
` (4 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2025-10-08 16:56 UTC (permalink / raw)
To: linux-xfs, aalbersh, cem, cmaiolino, djwong, hch, pchelkin,
pranav.tyagi03, sandeen
Source kernel commit: d8e1ea43e5a314bc01ec059ce93396639dcf9112
xfs_trans_alloc_empty can't return errors, so return the allocated
transaction directly instead of an output double pointer argument.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
db/attrset.c | 6 +-----
db/dquot.c | 4 +---
db/fsmap.c | 8 +-------
db/info.c | 8 +-------
db/namei.c | 4 +---
db/rdump.c | 7 +------
include/xfs_trans.h | 2 +-
libxfs/inode.c | 4 +---
libxfs/trans.c | 37 ++++++++++++++++++++++---------------
libxfs/xfs_refcount.c | 4 +---
repair/phase2.c | 6 +-----
repair/pptr.c | 4 ++--
repair/quotacheck.c | 9 ++-------
repair/rcbag.c | 8 ++------
repair/rmap.c | 4 +---
repair/rt.c | 10 ++--------
16 files changed, 41 insertions(+), 84 deletions(-)
diff --git a/db/attrset.c b/db/attrset.c
index e3ffb75aa4..273c202956 100644
--- a/db/attrset.c
+++ b/db/attrset.c
@@ -823,11 +823,7 @@
return 0;
}
- error = -libxfs_trans_alloc_empty(mp, &tp);
- if (error) {
- dbprintf(_("failed to allocate empty transaction\n"));
- return 0;
- }
+ tp = libxfs_trans_alloc_empty(mp);
error = -libxfs_iget(mp, NULL, iocur_top->ino, 0, &ip);
if (error) {
diff --git a/db/dquot.c b/db/dquot.c
index d2c76fd70b..c028d50e4c 100644
--- a/db/dquot.c
+++ b/db/dquot.c
@@ -92,9 +92,7 @@
xfs_ino_t ret = NULLFSINO;
int error;
- error = -libxfs_trans_alloc_empty(mp, &tp);
- if (error)
- return NULLFSINO;
+ tp = libxfs_trans_alloc_empty(mp);
if (xfs_has_metadir(mp)) {
error = -libxfs_dqinode_load_parent(tp, &dp);
diff --git a/db/fsmap.c b/db/fsmap.c
index ddbe4e6a3d..a59a4d1230 100644
--- a/db/fsmap.c
+++ b/db/fsmap.c
@@ -133,13 +133,7 @@
struct xfs_btree_cur *bt_cur;
int error;
- error = -libxfs_trans_alloc_empty(mp, &tp);
- if (error) {
- dbprintf(
- _("Cannot alloc transaction to look up rtgroup %u rmap inode\n"),
- rtg_rgno(rtg));
- return error;
- }
+ tp = libxfs_trans_alloc_empty(mp);
error = -libxfs_rtginode_load_parent(tp);
if (error) {
diff --git a/db/info.c b/db/info.c
index 6ad3e23832..9c233c9c0e 100644
--- a/db/info.c
+++ b/db/info.c
@@ -174,13 +174,7 @@
xfs_filblks_t used = 0;
int error;
- error = -libxfs_trans_alloc_empty(mp, &tp);
- if (error) {
- dbprintf(
- _("Cannot alloc transaction to look up rtgroup %u rmap inode\n"),
- rtg_rgno(rtg));
- return;
- }
+ tp = libxfs_trans_alloc_empty(mp);
error = -libxfs_rtginode_load_parent(tp);
if (error) {
diff --git a/db/namei.c b/db/namei.c
index 1d9581c323..0a50ec87df 100644
--- a/db/namei.c
+++ b/db/namei.c
@@ -94,9 +94,7 @@
unsigned int i;
int error;
- error = -libxfs_trans_alloc_empty(mp, &tp);
- if (error)
- return error;
+ tp = libxfs_trans_alloc_empty(mp);
error = -libxfs_iget(mp, tp, ino, 0, &dp);
if (error)
diff --git a/db/rdump.c b/db/rdump.c
index a50df4b8c7..599d0727e7 100644
--- a/db/rdump.c
+++ b/db/rdump.c
@@ -926,15 +926,10 @@
set_cur_inode(mp->m_sb.sb_rootino);
}
- ret = -libxfs_trans_alloc_empty(mp, &tp);
- if (ret) {
- dbprintf(_("allocating state: %s\n"), strerror(ret));
- goto out_pbuf;
- }
+ tp = libxfs_trans_alloc_empty(mp);
ret = rdump_file(tp, iocur_top->ino, destdir, pbuf);
libxfs_trans_cancel(tp);
-out_pbuf:
free(pbuf);
return ret;
}
diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index 248064019a..4f4bfff350 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -98,7 +98,7 @@
struct xfs_trans **tpp, int *nospace_error);
int libxfs_trans_alloc_rollable(struct xfs_mount *mp, uint blocks,
struct xfs_trans **tpp);
-int libxfs_trans_alloc_empty(struct xfs_mount *mp, struct xfs_trans **tpp);
+struct xfs_trans *libxfs_trans_alloc_empty(struct xfs_mount *mp);
int libxfs_trans_commit(struct xfs_trans *);
void libxfs_trans_cancel(struct xfs_trans *);
int libxfs_trans_reserve_more(struct xfs_trans *tp, uint blocks,
diff --git a/libxfs/inode.c b/libxfs/inode.c
index 0598a70ff5..1ce159fcc9 100644
--- a/libxfs/inode.c
+++ b/libxfs/inode.c
@@ -258,9 +258,7 @@
struct xfs_trans *tp;
int error;
- error = libxfs_trans_alloc_empty(mp, &tp);
- if (error)
- return error;
+ tp = libxfs_trans_alloc_empty(mp);
error = libxfs_trans_metafile_iget(tp, ino, metafile_type, ipp);
libxfs_trans_cancel(tp);
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 5c896ba166..64457d1710 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -247,18 +247,12 @@
return error;
}
-int
-libxfs_trans_alloc(
+static inline struct xfs_trans *
+__libxfs_trans_alloc(
struct xfs_mount *mp,
- struct xfs_trans_res *resp,
- unsigned int blocks,
- unsigned int rtextents,
- unsigned int flags,
- struct xfs_trans **tpp)
-
+ uint flags)
{
struct xfs_trans *tp;
- int error;
tp = kmem_cache_zalloc(xfs_trans_cache, 0);
tp->t_mountp = mp;
@@ -266,6 +260,22 @@
INIT_LIST_HEAD(&tp->t_dfops);
tp->t_highest_agno = NULLAGNUMBER;
+ return tp;
+}
+
+int
+libxfs_trans_alloc(
+ struct xfs_mount *mp,
+ struct xfs_trans_res *resp,
+ unsigned int blocks,
+ unsigned int rtextents,
+ unsigned int flags,
+ struct xfs_trans **tpp)
+
+{
+ struct xfs_trans *tp = __libxfs_trans_alloc(mp, flags);
+ int error;
+
error = xfs_trans_reserve(tp, resp, blocks, rtextents);
if (error) {
xfs_trans_cancel(tp);
@@ -290,14 +300,11 @@
* Note the zero-length reservation; this transaction MUST be cancelled
* without any dirty data.
*/
-int
+struct xfs_trans *
libxfs_trans_alloc_empty(
- struct xfs_mount *mp,
- struct xfs_trans **tpp)
+ struct xfs_mount *mp)
{
- struct xfs_trans_res resv = {0};
-
- return xfs_trans_alloc(mp, &resv, 0, 0, XFS_TRANS_NO_WRITECOUNT, tpp);
+ return __libxfs_trans_alloc(mp, XFS_TRANS_NO_WRITECOUNT);
}
/*
diff --git a/libxfs/xfs_refcount.c b/libxfs/xfs_refcount.c
index 1000bab245..4d31c3379d 100644
--- a/libxfs/xfs_refcount.c
+++ b/libxfs/xfs_refcount.c
@@ -2097,9 +2097,7 @@
* recording the CoW debris we cancel the (empty) transaction
* and everything goes away cleanly.
*/
- error = xfs_trans_alloc_empty(mp, &tp);
- if (error)
- return error;
+ tp = xfs_trans_alloc_empty(mp);
if (isrt) {
xfs_rtgroup_lock(to_rtg(xg), XFS_RTGLOCK_REFCOUNT);
diff --git a/repair/phase2.c b/repair/phase2.c
index e249980527..fc96f9c422 100644
--- a/repair/phase2.c
+++ b/repair/phase2.c
@@ -296,11 +296,7 @@
* there while we try to make a per-AG reservation with the new
* geometry.
*/
- error = -libxfs_trans_alloc_empty(mp, &tp);
- if (error)
- do_error(
- _("Cannot reserve resources for upgrade check, err=%d.\n"),
- error);
+ tp = libxfs_trans_alloc_empty(mp);
error = -libxfs_ialloc_read_agi(pag, tp, 0, &agi_bp);
if (error)
diff --git a/repair/pptr.c b/repair/pptr.c
index ac0a9c618b..6a9e072b36 100644
--- a/repair/pptr.c
+++ b/repair/pptr.c
@@ -1217,7 +1217,7 @@
fscan->have_garbage = false;
fscan->nr_file_pptrs = 0;
- libxfs_trans_alloc_empty(ip->i_mount, &tp);
+ tp = libxfs_trans_alloc_empty(ip->i_mount);
error = xattr_walk(tp, ip, examine_xattr, fscan);
if (tp)
libxfs_trans_cancel(tp);
@@ -1417,7 +1417,7 @@
do_error("init garbage pptr names failed: %s\n",
strerror(error));
- libxfs_trans_alloc_empty(ip->i_mount, &tp);
+ tp = libxfs_trans_alloc_empty(ip->i_mount);
error = xattr_walk(tp, ip, erase_pptrs, &fscan);
if (tp)
libxfs_trans_cancel(tp);
diff --git a/repair/quotacheck.c b/repair/quotacheck.c
index df6cde2d58..f4c0314177 100644
--- a/repair/quotacheck.c
+++ b/repair/quotacheck.c
@@ -437,9 +437,7 @@
if (!dquots || !chkd_flags)
return;
- error = -libxfs_trans_alloc_empty(mp, &tp);
- if (error)
- do_error(_("could not alloc transaction to open quota file\n"));
+ tp = libxfs_trans_alloc_empty(mp);
ino = get_quota_inode(type);
error = -libxfs_trans_metafile_iget(tp, ino, metafile_type, &ip);
@@ -679,9 +677,7 @@
struct xfs_inode *dp = NULL;
int error, err2;
- error = -libxfs_trans_alloc_empty(mp, &tp);
- if (error)
- goto out;
+ tp = libxfs_trans_alloc_empty(mp);
error = -libxfs_dqinode_load_parent(tp, &dp);
if (error)
@@ -698,7 +694,6 @@
libxfs_irele(dp);
out_cancel:
libxfs_trans_cancel(tp);
-out:
if (error) {
switch (error) {
case EFSCORRUPTED:
diff --git a/repair/rcbag.c b/repair/rcbag.c
index 21732b65c6..d7addbf58e 100644
--- a/repair/rcbag.c
+++ b/repair/rcbag.c
@@ -95,9 +95,7 @@
int has;
int error;
- error = -libxfs_trans_alloc_empty(mp, &tp);
- if (error)
- do_error(_("allocating tx for refcount bag update\n"));
+ tp = libxfs_trans_alloc_empty(mp);
cur = rcbagbt_mem_cursor(mp, tp, &bag->xfbtree);
error = rcbagbt_lookup_eq(cur, rmap, &has);
@@ -217,9 +215,7 @@
int has;
int error;
- error = -libxfs_trans_alloc_empty(mp, &tp);
- if (error)
- do_error(_("allocating tx for refcount bag update\n"));
+ tp = libxfs_trans_alloc_empty(mp);
/* go to the right edge of the tree */
cur = rcbagbt_mem_cursor(mp, tp, &bag->xfbtree);
diff --git a/repair/rmap.c b/repair/rmap.c
index 97510dd875..e89bd32d63 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -323,9 +323,7 @@
int error;
xfbt = &rmaps_for_group(isrt, agno)->ar_xfbtree;
- error = -libxfs_trans_alloc_empty(mp, &tp);
- if (error)
- do_error(_("allocating tx for in-memory rmap update\n"));
+ tp = libxfs_trans_alloc_empty(mp);
error = rmap_init_mem_cursor(mp, tp, isrt, agno, &rmcur);
if (error)
diff --git a/repair/rt.c b/repair/rt.c
index 1ac2bf6fc4..781d896844 100644
--- a/repair/rt.c
+++ b/repair/rt.c
@@ -301,10 +301,7 @@
if (rtg->rtg_inodes[type])
goto out_rtg;
- error = -libxfs_trans_alloc_empty(mp, &tp);
- if (error)
- goto out_rtg;
-
+ tp = libxfs_trans_alloc_empty(mp);
error = -libxfs_rtginode_load(rtg, type, tp);
if (error)
@@ -497,9 +494,7 @@
int error, err2;
int i;
- error = -libxfs_trans_alloc_empty(mp, &tp);
- if (error)
- goto out;
+ tp = libxfs_trans_alloc_empty(mp);
if (xfs_has_rtgroups(mp) && mp->m_sb.sb_rgcount > 0) {
error = -libxfs_rtginode_load_parent(tp);
if (error)
@@ -516,7 +511,6 @@
out_cancel:
libxfs_trans_cancel(tp);
-out:
if (xfs_has_rtgroups(mp) && error) {
/*
* Old xfs_repair didn't complain if rtbitmaps didn't load
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 8/11] [PATCH] xfs: improve the xg_active_ref check in xfs_group_free
2025-10-08 16:41 [PATCH 0/12] xfsprogs: libxfs sync v6.17 Andrey Albershteyn
` (6 preceding siblings ...)
2025-10-08 16:56 ` [PATCH 7/11] [PATCH] xfs: return the allocated transaction from xfs_trans_alloc_empty Christoph Hellwig
@ 2025-10-08 16:56 ` Christoph Hellwig
2025-10-08 16:56 ` [PATCH 9/11] [PATCH] fs/xfs: replace strncpy with memtostr_pad() Pranav Tyagi
` (3 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2025-10-08 16:56 UTC (permalink / raw)
To: linux-xfs, aalbersh, cem, cmaiolino, djwong, hch, pchelkin,
pranav.tyagi03, sandeen
Source kernel commit: 59655147ec34fb72cc090ca4ee688ece05ffac56
Split up the XFS_IS_CORRUPT statement so that it immediately shows
if the reference counter overflowed or underflowed.
I ran into this quite a bit when developing the zoned allocator, and had
to reapply the patch for some work recently. We might as well just apply
it upstream given that freeing group is far removed from performance
critical code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
libxfs/xfs_group.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libxfs/xfs_group.c b/libxfs/xfs_group.c
index b7ac9dbd8e..03c508242d 100644
--- a/libxfs/xfs_group.c
+++ b/libxfs/xfs_group.c
@@ -170,7 +170,8 @@
/* drop the mount's active reference */
xfs_group_rele(xg);
- XFS_IS_CORRUPT(mp, atomic_read(&xg->xg_active_ref) != 0);
+ XFS_IS_CORRUPT(mp, atomic_read(&xg->xg_active_ref) > 0);
+ XFS_IS_CORRUPT(mp, atomic_read(&xg->xg_active_ref) < 0);
kfree_rcu_mightsleep(xg);
}
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 9/11] [PATCH] fs/xfs: replace strncpy with memtostr_pad()
2025-10-08 16:41 [PATCH 0/12] xfsprogs: libxfs sync v6.17 Andrey Albershteyn
` (7 preceding siblings ...)
2025-10-08 16:56 ` [PATCH 8/11] [PATCH] xfs: improve the xg_active_ref check in xfs_group_free Christoph Hellwig
@ 2025-10-08 16:56 ` Pranav Tyagi
2025-10-08 16:56 ` [PATCH 10/11] [PATCH] xfs: don't use a xfs_log_iovec for ri_buf in log recovery Christoph Hellwig
` (2 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Pranav Tyagi @ 2025-10-08 16:56 UTC (permalink / raw)
To: linux-xfs, aalbersh, cem, cmaiolino, djwong, hch, pchelkin,
pranav.tyagi03, sandeen
Source kernel commit: f4a3f01e8e451fb3cb444a95a59964f4bc746902
Replace the deprecated strncpy() with memtostr_pad(). This also avoids
the need for separate zeroing using memset(). Mark sb_fname buffer with
__nonstring as its size is XFSLABEL_MAX and so no terminating NULL for
sb_fname.
Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
db/metadump.c | 2 +-
include/platform_defs.h | 6 ++++++
libxfs/xfs_format.h | 2 +-
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/db/metadump.c b/db/metadump.c
index 34f2d61700..24eb99da17 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -2989,7 +2989,7 @@
if (metadump.obfuscate) {
struct xfs_sb *sb = iocur_top->data;
memset(sb->sb_fname, 'L',
- min(strlen(sb->sb_fname), sizeof(sb->sb_fname)));
+ strnlen(sb->sb_fname, sizeof(sb->sb_fname)));
iocur_top->need_crc = 1;
}
if (write_buf(iocur_top))
diff --git a/include/platform_defs.h b/include/platform_defs.h
index fa66551d99..7b4a1a6255 100644
--- a/include/platform_defs.h
+++ b/include/platform_defs.h
@@ -296,4 +296,10 @@
#define cmp_int(l, r) ((l > r) - (l < r))
+#if __has_attribute(__nonstring__)
+# define __nonstring __attribute__((__nonstring__))
+#else
+# define __nonstring
+#endif
+
#endif /* __XFS_PLATFORM_DEFS_H__ */
diff --git a/libxfs/xfs_format.h b/libxfs/xfs_format.h
index 9566a76233..779dac59b1 100644
--- a/libxfs/xfs_format.h
+++ b/libxfs/xfs_format.h
@@ -112,7 +112,7 @@
uint16_t sb_sectsize; /* volume sector size, bytes */
uint16_t sb_inodesize; /* inode size, bytes */
uint16_t sb_inopblock; /* inodes per block */
- char sb_fname[XFSLABEL_MAX]; /* file system name */
+ char sb_fname[XFSLABEL_MAX] __nonstring; /* file system name */
uint8_t sb_blocklog; /* log2 of sb_blocksize */
uint8_t sb_sectlog; /* log2 of sb_sectsize */
uint8_t sb_inodelog; /* log2 of sb_inodesize */
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 10/11] [PATCH] xfs: don't use a xfs_log_iovec for ri_buf in log recovery
2025-10-08 16:41 [PATCH 0/12] xfsprogs: libxfs sync v6.17 Andrey Albershteyn
` (8 preceding siblings ...)
2025-10-08 16:56 ` [PATCH 9/11] [PATCH] fs/xfs: replace strncpy with memtostr_pad() Pranav Tyagi
@ 2025-10-08 16:56 ` Christoph Hellwig
2025-10-08 16:57 ` [PATCH 11/11] [PATCH] xfs: do not propagate ENODATA disk errors into xattr code Eric Sandeen
2025-10-08 20:39 ` [PATCH 0/12] xfsprogs: libxfs sync v6.17 Darrick J. Wong
11 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2025-10-08 16:56 UTC (permalink / raw)
To: linux-xfs, aalbersh, cem, cmaiolino, djwong, hch, pchelkin,
pranav.tyagi03, sandeen
Source kernel commit: ded74fddcaf685a9440c5612f7831d0c4c1473ca
ri_buf just holds a pointer/len pair and is not a log iovec used for
writing to the log. Switch to use a kvec instead.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
include/platform_defs.h | 5 ++++
libxfs/xfs_log_recover.h | 4 +--
libxlog/xfs_log_recover.c | 14 +++++------
logprint/log_print_all.c | 59 ++++++++++++++++++++++++-------------------------
logprint/log_redo.c | 52 +++++++++++++++++++++----------------------
5 files changed, 70 insertions(+), 64 deletions(-)
diff --git a/include/platform_defs.h b/include/platform_defs.h
index 7b4a1a6255..da966490b0 100644
--- a/include/platform_defs.h
+++ b/include/platform_defs.h
@@ -302,4 +302,9 @@
# define __nonstring
#endif
+struct kvec {
+ void *iov_base;
+ size_t iov_len;
+};
+
#endif /* __XFS_PLATFORM_DEFS_H__ */
diff --git a/libxfs/xfs_log_recover.h b/libxfs/xfs_log_recover.h
index 66c7916fb5..95de230950 100644
--- a/libxfs/xfs_log_recover.h
+++ b/libxfs/xfs_log_recover.h
@@ -104,7 +104,7 @@
struct list_head ri_list;
int ri_cnt; /* count of regions found */
int ri_total; /* total regions */
- struct xfs_log_iovec *ri_buf; /* ptr to regions buffer */
+ struct kvec *ri_buf; /* ptr to regions buffer */
const struct xlog_recover_item_ops *ri_ops;
};
@@ -117,7 +117,7 @@
struct list_head r_itemq; /* q for items */
};
-#define ITEM_TYPE(i) (*(unsigned short *)(i)->ri_buf[0].i_addr)
+#define ITEM_TYPE(i) (*(unsigned short *)(i)->ri_buf[0].iov_base)
#define XLOG_RECOVER_CRCPASS 0
#define XLOG_RECOVER_PASS1 1
diff --git a/libxlog/xfs_log_recover.c b/libxlog/xfs_log_recover.c
index 275593a3ac..7ef43956e9 100644
--- a/libxlog/xfs_log_recover.c
+++ b/libxlog/xfs_log_recover.c
@@ -1034,13 +1034,13 @@
item = list_entry(trans->r_itemq.prev, struct xlog_recover_item,
ri_list);
- old_ptr = item->ri_buf[item->ri_cnt-1].i_addr;
- old_len = item->ri_buf[item->ri_cnt-1].i_len;
+ old_ptr = item->ri_buf[item->ri_cnt-1].iov_base;
+ old_len = item->ri_buf[item->ri_cnt-1].iov_len;
ptr = krealloc(old_ptr, len+old_len, 0);
memcpy(&ptr[old_len], dp, len); /* d, s, l */
- item->ri_buf[item->ri_cnt-1].i_len += len;
- item->ri_buf[item->ri_cnt-1].i_addr = ptr;
+ item->ri_buf[item->ri_cnt-1].iov_len += len;
+ item->ri_buf[item->ri_cnt-1].iov_base = ptr;
trace_xfs_log_recover_item_add_cont(log, trans, item, 0);
return 0;
}
@@ -1117,8 +1117,8 @@
}
ASSERT(item->ri_total > item->ri_cnt);
/* Description region is ri_buf[0] */
- item->ri_buf[item->ri_cnt].i_addr = ptr;
- item->ri_buf[item->ri_cnt].i_len = len;
+ item->ri_buf[item->ri_cnt].iov_base = ptr;
+ item->ri_buf[item->ri_cnt].iov_len = len;
item->ri_cnt++;
trace_xfs_log_recover_item_add(log, trans, item, 0);
return 0;
@@ -1140,7 +1140,7 @@
/* Free the regions in the item. */
list_del(&item->ri_list);
for (i = 0; i < item->ri_cnt; i++)
- kfree(item->ri_buf[i].i_addr);
+ kfree(item->ri_buf[i].iov_base);
/* Free the item itself */
kfree(item->ri_buf);
kfree(item);
diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
index 1498ef9724..39946f32d4 100644
--- a/logprint/log_print_all.c
+++ b/logprint/log_print_all.c
@@ -78,7 +78,7 @@
xfs_daddr_t blkno;
struct xfs_disk_dquot *ddq;
- f = (xfs_buf_log_format_t *)item->ri_buf[0].i_addr;
+ f = (xfs_buf_log_format_t *)item->ri_buf[0].iov_base;
printf(" ");
ASSERT(f->blf_type == XFS_LI_BUF);
printf(_("BUF: #regs:%d start blkno:0x%llx len:%d bmap size:%d flags:0x%x\n"),
@@ -87,8 +87,8 @@
num = f->blf_size-1;
i = 1;
while (num-- > 0) {
- p = item->ri_buf[i].i_addr;
- len = item->ri_buf[i].i_len;
+ p = item->ri_buf[i].iov_base;
+ len = item->ri_buf[i].iov_len;
i++;
if (blkno == 0) { /* super block */
struct xfs_dsb *dsb = (struct xfs_dsb *)p;
@@ -185,7 +185,7 @@
{
xfs_qoff_logformat_t *qoff_f;
- qoff_f = (xfs_qoff_logformat_t *)item->ri_buf[0].i_addr;
+ qoff_f = (xfs_qoff_logformat_t *)item->ri_buf[0].iov_base;
ASSERT(qoff_f);
printf(_("\tQUOTAOFF: #regs:%d type:"), qoff_f->qf_size);
@@ -205,10 +205,10 @@
xfs_dq_logformat_t *f;
struct xfs_disk_dquot *d;
- f = (xfs_dq_logformat_t *)item->ri_buf[0].i_addr;
+ f = (xfs_dq_logformat_t *)item->ri_buf[0].iov_base;
ASSERT(f);
ASSERT(f->qlf_len == 1);
- d = (struct xfs_disk_dquot *)item->ri_buf[1].i_addr;
+ d = (struct xfs_disk_dquot *)item->ri_buf[1].iov_base;
printf(_("\tDQUOT: #regs:%d blkno:%lld boffset:%u id: %d\n"),
f->qlf_size, (long long)f->qlf_blkno, f->qlf_boffset, f->qlf_id);
if (!print_quota)
@@ -288,21 +288,22 @@
int hasdata;
int hasattr;
- ASSERT(item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format_32) ||
- item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format));
- f = xfs_inode_item_format_convert(item->ri_buf[0].i_addr, item->ri_buf[0].i_len, &f_buf);
+ ASSERT(item->ri_buf[0].iov_len == sizeof(struct xfs_inode_log_format_32) ||
+ item->ri_buf[0].iov_len == sizeof(struct xfs_inode_log_format));
+ f = xfs_inode_item_format_convert(item->ri_buf[0].iov_base,
+ item->ri_buf[0].iov_len, &f_buf);
printf(_(" INODE: #regs:%d ino:0x%llx flags:0x%x dsize:%d\n"),
- f->ilf_size, (unsigned long long)f->ilf_ino, f->ilf_fields,
- f->ilf_dsize);
+ f->ilf_size, (unsigned long long)f->ilf_ino, f->ilf_fields,
+ f->ilf_dsize);
/* core inode comes 2nd */
/* ASSERT len vs xfs_log_dinode_size() for V3 or V2 inodes */
- ASSERT(item->ri_buf[1].i_len ==
+ ASSERT(item->ri_buf[1].iov_len ==
offsetof(struct xfs_log_dinode, di_next_unlinked) ||
- item->ri_buf[1].i_len == sizeof(struct xfs_log_dinode));
+ item->ri_buf[1].iov_len == sizeof(struct xfs_log_dinode));
xlog_recover_print_inode_core((struct xfs_log_dinode *)
- item->ri_buf[1].i_addr);
+ item->ri_buf[1].iov_base);
hasdata = (f->ilf_fields & XFS_ILOG_DFORK) != 0;
hasattr = (f->ilf_fields & XFS_ILOG_AFORK) != 0;
@@ -312,22 +313,22 @@
ASSERT(f->ilf_size == 3 + hasattr);
printf(_(" DATA FORK EXTENTS inode data:\n"));
if (print_inode && print_data)
- xlog_recover_print_data(item->ri_buf[2].i_addr,
- item->ri_buf[2].i_len);
+ xlog_recover_print_data(item->ri_buf[2].iov_base,
+ item->ri_buf[2].iov_len);
break;
case XFS_ILOG_DBROOT:
ASSERT(f->ilf_size == 3 + hasattr);
printf(_(" DATA FORK BTREE inode data:\n"));
if (print_inode && print_data)
- xlog_recover_print_data(item->ri_buf[2].i_addr,
- item->ri_buf[2].i_len);
+ xlog_recover_print_data(item->ri_buf[2].iov_base,
+ item->ri_buf[2].iov_len);
break;
case XFS_ILOG_DDATA:
ASSERT(f->ilf_size == 3 + hasattr);
printf(_(" DATA FORK LOCAL inode data:\n"));
if (print_inode && print_data)
- xlog_recover_print_data(item->ri_buf[2].i_addr,
- item->ri_buf[2].i_len);
+ xlog_recover_print_data(item->ri_buf[2].iov_base,
+ item->ri_buf[2].iov_len);
break;
case XFS_ILOG_DEV:
ASSERT(f->ilf_size == 2 + hasattr);
@@ -353,24 +354,24 @@
printf(_(" ATTR FORK EXTENTS inode data:\n"));
if (print_inode && print_data)
xlog_recover_print_data(
- item->ri_buf[attr_index].i_addr,
- item->ri_buf[attr_index].i_len);
+ item->ri_buf[attr_index].iov_base,
+ item->ri_buf[attr_index].iov_len);
break;
case XFS_ILOG_ABROOT:
ASSERT(f->ilf_size == 3 + hasdata);
printf(_(" ATTR FORK BTREE inode data:\n"));
if (print_inode && print_data)
xlog_recover_print_data(
- item->ri_buf[attr_index].i_addr,
- item->ri_buf[attr_index].i_len);
+ item->ri_buf[attr_index].iov_base,
+ item->ri_buf[attr_index].iov_len);
break;
case XFS_ILOG_ADATA:
ASSERT(f->ilf_size == 3 + hasdata);
printf(_(" ATTR FORK LOCAL inode data:\n"));
if (print_inode && print_data)
xlog_recover_print_data(
- item->ri_buf[attr_index].i_addr,
- item->ri_buf[attr_index].i_len);
+ item->ri_buf[attr_index].iov_base,
+ item->ri_buf[attr_index].iov_len);
break;
default:
xlog_panic("%s: illegal inode log flag", __FUNCTION__);
@@ -385,7 +386,7 @@
{
struct xfs_icreate_log *icl;
- icl = (struct xfs_icreate_log *)item->ri_buf[0].i_addr;
+ icl = (struct xfs_icreate_log *)item->ri_buf[0].iov_base;
printf(_(" ICR: #ag: %d agbno: 0x%x len: %d\n"
" cnt: %d isize: %d gen: 0x%x\n"),
@@ -549,8 +550,8 @@
*/
printf(_(": cnt:%d total:%d "), item->ri_cnt, item->ri_total);
for (i=0; i<item->ri_cnt; i++) {
- printf(_("a:0x%lx len:%d "),
- (long)item->ri_buf[i].i_addr, item->ri_buf[i].i_len);
+ printf(_("a:0x%lx len:%zu "),
+ (long)item->ri_buf[i].iov_base, item->ri_buf[i].iov_len);
}
printf("\n");
xlog_recover_print_logitem(item);
diff --git a/logprint/log_redo.c b/logprint/log_redo.c
index 89d7448342..f5bac21d35 100644
--- a/logprint/log_redo.c
+++ b/logprint/log_redo.c
@@ -143,8 +143,8 @@
int i;
uint src_len, dst_len;
- src_f = (xfs_efi_log_format_t *)item->ri_buf[0].i_addr;
- src_len = item->ri_buf[0].i_len;
+ src_f = (xfs_efi_log_format_t *)item->ri_buf[0].iov_base;
+ src_len = item->ri_buf[0].iov_len;
/*
* An xfs_efi_log_format structure contains a variable length array
* as the last field.
@@ -229,7 +229,7 @@
const char *item_name = "EFD?";
xfs_efd_log_format_t *f;
- f = (xfs_efd_log_format_t *)item->ri_buf[0].i_addr;
+ f = (xfs_efd_log_format_t *)item->ri_buf[0].iov_base;
switch (f->efd_type) {
case XFS_LI_EFD: item_name = "EFD"; break;
@@ -355,8 +355,8 @@
char *src_f;
uint src_len;
- src_f = item->ri_buf[0].i_addr;
- src_len = item->ri_buf[0].i_len;
+ src_f = item->ri_buf[0].iov_base;
+ src_len = item->ri_buf[0].iov_len;
xlog_print_trans_rui(&src_f, src_len, 0);
}
@@ -406,7 +406,7 @@
{
char *f;
- f = item->ri_buf[0].i_addr;
+ f = item->ri_buf[0].iov_base;
xlog_print_trans_rud(&f, sizeof(struct xfs_rud_log_format));
}
@@ -516,8 +516,8 @@
char *src_f;
uint src_len;
- src_f = item->ri_buf[0].i_addr;
- src_len = item->ri_buf[0].i_len;
+ src_f = item->ri_buf[0].iov_base;
+ src_len = item->ri_buf[0].iov_len;
xlog_print_trans_cui(&src_f, src_len, 0);
}
@@ -563,7 +563,7 @@
{
char *f;
- f = item->ri_buf[0].i_addr;
+ f = item->ri_buf[0].iov_base;
xlog_print_trans_cud(&f, sizeof(struct xfs_cud_log_format));
}
@@ -667,8 +667,8 @@
char *src_f;
uint src_len;
- src_f = item->ri_buf[0].i_addr;
- src_len = item->ri_buf[0].i_len;
+ src_f = item->ri_buf[0].iov_base;
+ src_len = item->ri_buf[0].iov_len;
xlog_print_trans_bui(&src_f, src_len, 0);
}
@@ -707,7 +707,7 @@
{
char *f;
- f = item->ri_buf[0].i_addr;
+ f = item->ri_buf[0].iov_base;
xlog_print_trans_bud(&f, sizeof(struct xfs_bud_log_format));
}
@@ -954,8 +954,8 @@
unsigned int new_value_len = 0;
int region = 0;
- src_f = (struct xfs_attri_log_format *)item->ri_buf[0].i_addr;
- src_len = item->ri_buf[region].i_len;
+ src_f = (struct xfs_attri_log_format *)item->ri_buf[0].iov_base;
+ src_len = item->ri_buf[region].iov_len;
/*
* An xfs_attri_log_format structure contains a attribute name and
@@ -996,17 +996,17 @@
if (name_len > 0) {
region++;
printf(_("ATTRI: name len:%u\n"), name_len);
- print_or_dump((char *)item->ri_buf[region].i_addr,
+ print_or_dump((char *)item->ri_buf[region].iov_base,
name_len);
- name_ptr = item->ri_buf[region].i_addr;
+ name_ptr = item->ri_buf[region].iov_base;
}
if (new_name_len > 0) {
region++;
printf(_("ATTRI: newname len:%u\n"), new_name_len);
- print_or_dump((char *)item->ri_buf[region].i_addr,
+ print_or_dump((char *)item->ri_buf[region].iov_base,
new_name_len);
- new_name_ptr = item->ri_buf[region].i_addr;
+ new_name_ptr = item->ri_buf[region].iov_base;
}
if (value_len > 0) {
@@ -1014,8 +1014,8 @@
region++;
printf(_("ATTRI: value len:%u\n"), value_len);
- print_or_dump((char *)item->ri_buf[region].i_addr, len);
- value_ptr = item->ri_buf[region].i_addr;
+ print_or_dump((char *)item->ri_buf[region].iov_base, len);
+ value_ptr = item->ri_buf[region].iov_base;
}
if (new_value_len > 0) {
@@ -1023,8 +1023,8 @@
region++;
printf(_("ATTRI: newvalue len:%u\n"), new_value_len);
- print_or_dump((char *)item->ri_buf[region].i_addr, len);
- new_value_ptr = item->ri_buf[region].i_addr;
+ print_or_dump((char *)item->ri_buf[region].iov_base, len);
+ new_value_ptr = item->ri_buf[region].iov_base;
}
if (src_f->alfi_attr_filter & XFS_ATTR_PARENT)
@@ -1065,7 +1065,7 @@
{
struct xfs_attrd_log_format *f;
- f = (struct xfs_attrd_log_format *)item->ri_buf[0].i_addr;
+ f = (struct xfs_attrd_log_format *)item->ri_buf[0].iov_base;
printf(_(" ATTRD: #regs: %d id: 0x%llx\n"),
f->alfd_size,
@@ -1156,8 +1156,8 @@
char *src_f;
uint src_len;
- src_f = item->ri_buf[0].i_addr;
- src_len = item->ri_buf[0].i_len;
+ src_f = item->ri_buf[0].iov_base;
+ src_len = item->ri_buf[0].iov_len;
xlog_print_trans_xmi(&src_f, src_len, 0);
}
@@ -1196,6 +1196,6 @@
{
char *f;
- f = item->ri_buf[0].i_addr;
+ f = item->ri_buf[0].iov_base;
xlog_print_trans_xmd(&f, sizeof(struct xfs_xmd_log_format));
}
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 11/11] [PATCH] xfs: do not propagate ENODATA disk errors into xattr code
2025-10-08 16:41 [PATCH 0/12] xfsprogs: libxfs sync v6.17 Andrey Albershteyn
` (9 preceding siblings ...)
2025-10-08 16:56 ` [PATCH 10/11] [PATCH] xfs: don't use a xfs_log_iovec for ri_buf in log recovery Christoph Hellwig
@ 2025-10-08 16:57 ` Eric Sandeen
2025-10-08 20:39 ` [PATCH 0/12] xfsprogs: libxfs sync v6.17 Darrick J. Wong
11 siblings, 0 replies; 15+ messages in thread
From: Eric Sandeen @ 2025-10-08 16:57 UTC (permalink / raw)
To: linux-xfs, aalbersh, cem, cmaiolino, djwong, hch, pchelkin,
pranav.tyagi03, sandeen
Source kernel commit: ae668cd567a6a7622bc813ee0bb61c42bed61ba7
ENODATA (aka ENOATTR) has a very specific meaning in the xfs xattr code;
namely, that the requested attribute name could not be found.
However, a medium error from disk may also return ENODATA. At best,
this medium error may escape to userspace as "attribute not found"
when in fact it's an IO (disk) error.
At worst, we may oops in xfs_attr_leaf_get() when we do:
error = xfs_attr_leaf_hasname(args, &bp);
if (error == -ENOATTR) {
xfs_trans_brelse(args->trans, bp);
return error;
}
because an ENODATA/ENOATTR error from disk leaves us with a null bp,
and the xfs_trans_brelse will then null-deref it.
As discussed on the list, we really need to modify the lower level
IO functions to trap all disk errors and ensure that we don't let
unique errors like this leak up into higher xfs functions - many
like this should be remapped to EIO.
However, this patch directly addresses a reported bug in the xattr
code, and should be safe to backport to stable kernels. A larger-scope
patch to handle more unique errors at lower levels can follow later.
(Note, prior to 07120f1abdff we did not oops, but we did return the
wrong error code to userspace.)
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Fixes: 07120f1abdff ("xfs: Add xfs_has_attr and subroutines")
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
libxfs/xfs_attr_remote.c | 7 +++++++
libxfs/xfs_da_btree.c | 6 ++++++
2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/libxfs/xfs_attr_remote.c b/libxfs/xfs_attr_remote.c
index a048aa5f2c..82217baf40 100644
--- a/libxfs/xfs_attr_remote.c
+++ b/libxfs/xfs_attr_remote.c
@@ -434,6 +434,13 @@
0, &bp, &xfs_attr3_rmt_buf_ops);
if (xfs_metadata_is_sick(error))
xfs_dirattr_mark_sick(args->dp, XFS_ATTR_FORK);
+ /*
+ * ENODATA from disk implies a disk medium failure;
+ * ENODATA for xattrs means attribute not found, so
+ * disambiguate that here.
+ */
+ if (error == -ENODATA)
+ error = -EIO;
if (error)
return error;
diff --git a/libxfs/xfs_da_btree.c b/libxfs/xfs_da_btree.c
index 38f345a923..af3fcdf5e4 100644
--- a/libxfs/xfs_da_btree.c
+++ b/libxfs/xfs_da_btree.c
@@ -2829,6 +2829,12 @@
&bp, ops);
if (xfs_metadata_is_sick(error))
xfs_dirattr_mark_sick(dp, whichfork);
+ /*
+ * ENODATA from disk implies a disk medium failure; ENODATA for
+ * xattrs means attribute not found, so disambiguate that here.
+ */
+ if (error == -ENODATA && whichfork == XFS_ATTR_FORK)
+ error = -EIO;
if (error)
goto out_free;
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 0/12] xfsprogs: libxfs sync v6.17
2025-10-08 16:41 [PATCH 0/12] xfsprogs: libxfs sync v6.17 Andrey Albershteyn
` (10 preceding siblings ...)
2025-10-08 16:57 ` [PATCH 11/11] [PATCH] xfs: do not propagate ENODATA disk errors into xattr code Eric Sandeen
@ 2025-10-08 20:39 ` Darrick J. Wong
11 siblings, 0 replies; 15+ messages in thread
From: Darrick J. Wong @ 2025-10-08 20:39 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: linux-xfs, cem, cmaiolino, hch, pchelkin, pranav.tyagi03, sandeen
On Wed, Oct 08, 2025 at 06:41:17PM +0200, Andrey Albershteyn wrote:
> Hey all,
>
> This is libxfs sync with v6.17.
>
> Manual modifications diff in attachment.
For everything except patches 3-4,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
>
> --
> - Andrey
> diff --git a/db/attrset.c b/db/attrset.c
> index e3ffb75aa4..273c202956 100644
> --- a/db/attrset.c
> +++ b/db/attrset.c
> @@ -823,11 +823,7 @@
> return 0;
> }
>
> - error = -libxfs_trans_alloc_empty(mp, &tp);
> - if (error) {
> - dbprintf(_("failed to allocate empty transaction\n"));
> - return 0;
> - }
> + tp = libxfs_trans_alloc_empty(mp);
>
> error = -libxfs_iget(mp, NULL, iocur_top->ino, 0, &ip);
> if (error) {
> diff --git a/db/dquot.c b/db/dquot.c
> index d2c76fd70b..c028d50e4c 100644
> --- a/db/dquot.c
> +++ b/db/dquot.c
> @@ -92,9 +92,7 @@
> xfs_ino_t ret = NULLFSINO;
> int error;
>
> - error = -libxfs_trans_alloc_empty(mp, &tp);
> - if (error)
> - return NULLFSINO;
> + tp = libxfs_trans_alloc_empty(mp);
>
> if (xfs_has_metadir(mp)) {
> error = -libxfs_dqinode_load_parent(tp, &dp);
> diff --git a/db/fsmap.c b/db/fsmap.c
> index ddbe4e6a3d..a59a4d1230 100644
> --- a/db/fsmap.c
> +++ b/db/fsmap.c
> @@ -133,13 +133,7 @@
> struct xfs_btree_cur *bt_cur;
> int error;
>
> - error = -libxfs_trans_alloc_empty(mp, &tp);
> - if (error) {
> - dbprintf(
> - _("Cannot alloc transaction to look up rtgroup %u rmap inode\n"),
> - rtg_rgno(rtg));
> - return error;
> - }
> + tp = libxfs_trans_alloc_empty(mp);
>
> error = -libxfs_rtginode_load_parent(tp);
> if (error) {
> diff --git a/db/info.c b/db/info.c
> index 6ad3e23832..9c233c9c0e 100644
> --- a/db/info.c
> +++ b/db/info.c
> @@ -174,13 +174,7 @@
> xfs_filblks_t used = 0;
> int error;
>
> - error = -libxfs_trans_alloc_empty(mp, &tp);
> - if (error) {
> - dbprintf(
> - _("Cannot alloc transaction to look up rtgroup %u rmap inode\n"),
> - rtg_rgno(rtg));
> - return;
> - }
> + tp = libxfs_trans_alloc_empty(mp);
>
> error = -libxfs_rtginode_load_parent(tp);
> if (error) {
> diff --git a/db/metadump.c b/db/metadump.c
> index 34f2d61700..24eb99da17 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -2989,7 +2989,7 @@
> if (metadump.obfuscate) {
> struct xfs_sb *sb = iocur_top->data;
> memset(sb->sb_fname, 'L',
> - min(strlen(sb->sb_fname), sizeof(sb->sb_fname)));
> + strnlen(sb->sb_fname, sizeof(sb->sb_fname)));
> iocur_top->need_crc = 1;
> }
> if (write_buf(iocur_top))
> diff --git a/db/namei.c b/db/namei.c
> index 1d9581c323..0a50ec87df 100644
> --- a/db/namei.c
> +++ b/db/namei.c
> @@ -94,9 +94,7 @@
> unsigned int i;
> int error;
>
> - error = -libxfs_trans_alloc_empty(mp, &tp);
> - if (error)
> - return error;
> + tp = libxfs_trans_alloc_empty(mp);
>
> error = -libxfs_iget(mp, tp, ino, 0, &dp);
> if (error)
> diff --git a/db/rdump.c b/db/rdump.c
> index a50df4b8c7..599d0727e7 100644
> --- a/db/rdump.c
> +++ b/db/rdump.c
> @@ -926,15 +926,10 @@
> set_cur_inode(mp->m_sb.sb_rootino);
> }
>
> - ret = -libxfs_trans_alloc_empty(mp, &tp);
> - if (ret) {
> - dbprintf(_("allocating state: %s\n"), strerror(ret));
> - goto out_pbuf;
> - }
> + tp = libxfs_trans_alloc_empty(mp);
>
> ret = rdump_file(tp, iocur_top->ino, destdir, pbuf);
> libxfs_trans_cancel(tp);
> -out_pbuf:
> free(pbuf);
> return ret;
> }
> diff --git a/include/platform_defs.h b/include/platform_defs.h
> index 74a00583eb..da966490b0 100644
> --- a/include/platform_defs.h
> +++ b/include/platform_defs.h
> @@ -294,4 +294,17 @@
> __a > __b ? (__a - __b) : (__b - __a); \
> })
>
> +#define cmp_int(l, r) ((l > r) - (l < r))
> +
> +#if __has_attribute(__nonstring__)
> +# define __nonstring __attribute__((__nonstring__))
> +#else
> +# define __nonstring
> +#endif
> +
> +struct kvec {
> + void *iov_base;
> + size_t iov_len;
> +};
> +
> #endif /* __XFS_PLATFORM_DEFS_H__ */
> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
> index 248064019a..4f4bfff350 100644
> --- a/include/xfs_trans.h
> +++ b/include/xfs_trans.h
> @@ -98,7 +98,7 @@
> struct xfs_trans **tpp, int *nospace_error);
> int libxfs_trans_alloc_rollable(struct xfs_mount *mp, uint blocks,
> struct xfs_trans **tpp);
> -int libxfs_trans_alloc_empty(struct xfs_mount *mp, struct xfs_trans **tpp);
> +struct xfs_trans *libxfs_trans_alloc_empty(struct xfs_mount *mp);
> int libxfs_trans_commit(struct xfs_trans *);
> void libxfs_trans_cancel(struct xfs_trans *);
> int libxfs_trans_reserve_more(struct xfs_trans *tp, uint blocks,
> diff --git a/libxfs/inode.c b/libxfs/inode.c
> index 0598a70ff5..1ce159fcc9 100644
> --- a/libxfs/inode.c
> +++ b/libxfs/inode.c
> @@ -258,9 +258,7 @@
> struct xfs_trans *tp;
> int error;
>
> - error = libxfs_trans_alloc_empty(mp, &tp);
> - if (error)
> - return error;
> + tp = libxfs_trans_alloc_empty(mp);
>
> error = libxfs_trans_metafile_iget(tp, ino, metafile_type, ipp);
> libxfs_trans_cancel(tp);
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 5c896ba166..64457d1710 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -247,18 +247,12 @@
> return error;
> }
>
> -int
> -libxfs_trans_alloc(
> +static inline struct xfs_trans *
> +__libxfs_trans_alloc(
> struct xfs_mount *mp,
> - struct xfs_trans_res *resp,
> - unsigned int blocks,
> - unsigned int rtextents,
> - unsigned int flags,
> - struct xfs_trans **tpp)
> -
> + uint flags)
> {
> struct xfs_trans *tp;
> - int error;
>
> tp = kmem_cache_zalloc(xfs_trans_cache, 0);
> tp->t_mountp = mp;
> @@ -266,6 +260,22 @@
> INIT_LIST_HEAD(&tp->t_dfops);
> tp->t_highest_agno = NULLAGNUMBER;
>
> + return tp;
> +}
> +
> +int
> +libxfs_trans_alloc(
> + struct xfs_mount *mp,
> + struct xfs_trans_res *resp,
> + unsigned int blocks,
> + unsigned int rtextents,
> + unsigned int flags,
> + struct xfs_trans **tpp)
> +
> +{
> + struct xfs_trans *tp = __libxfs_trans_alloc(mp, flags);
> + int error;
> +
> error = xfs_trans_reserve(tp, resp, blocks, rtextents);
> if (error) {
> xfs_trans_cancel(tp);
> @@ -290,14 +300,11 @@
> * Note the zero-length reservation; this transaction MUST be cancelled
> * without any dirty data.
> */
> -int
> +struct xfs_trans *
> libxfs_trans_alloc_empty(
> - struct xfs_mount *mp,
> - struct xfs_trans **tpp)
> + struct xfs_mount *mp)
> {
> - struct xfs_trans_res resv = {0};
> -
> - return xfs_trans_alloc(mp, &resv, 0, 0, XFS_TRANS_NO_WRITECOUNT, tpp);
> + return __libxfs_trans_alloc(mp, XFS_TRANS_NO_WRITECOUNT);
> }
>
> /*
> diff --git a/libxlog/xfs_log_recover.c b/libxlog/xfs_log_recover.c
> index 275593a3ac..7ef43956e9 100644
> --- a/libxlog/xfs_log_recover.c
> +++ b/libxlog/xfs_log_recover.c
> @@ -1034,13 +1034,13 @@
> item = list_entry(trans->r_itemq.prev, struct xlog_recover_item,
> ri_list);
>
> - old_ptr = item->ri_buf[item->ri_cnt-1].i_addr;
> - old_len = item->ri_buf[item->ri_cnt-1].i_len;
> + old_ptr = item->ri_buf[item->ri_cnt-1].iov_base;
> + old_len = item->ri_buf[item->ri_cnt-1].iov_len;
>
> ptr = krealloc(old_ptr, len+old_len, 0);
> memcpy(&ptr[old_len], dp, len); /* d, s, l */
> - item->ri_buf[item->ri_cnt-1].i_len += len;
> - item->ri_buf[item->ri_cnt-1].i_addr = ptr;
> + item->ri_buf[item->ri_cnt-1].iov_len += len;
> + item->ri_buf[item->ri_cnt-1].iov_base = ptr;
> trace_xfs_log_recover_item_add_cont(log, trans, item, 0);
> return 0;
> }
> @@ -1117,8 +1117,8 @@
> }
> ASSERT(item->ri_total > item->ri_cnt);
> /* Description region is ri_buf[0] */
> - item->ri_buf[item->ri_cnt].i_addr = ptr;
> - item->ri_buf[item->ri_cnt].i_len = len;
> + item->ri_buf[item->ri_cnt].iov_base = ptr;
> + item->ri_buf[item->ri_cnt].iov_len = len;
> item->ri_cnt++;
> trace_xfs_log_recover_item_add(log, trans, item, 0);
> return 0;
> @@ -1140,7 +1140,7 @@
> /* Free the regions in the item. */
> list_del(&item->ri_list);
> for (i = 0; i < item->ri_cnt; i++)
> - kfree(item->ri_buf[i].i_addr);
> + kfree(item->ri_buf[i].iov_base);
> /* Free the item itself */
> kfree(item->ri_buf);
> kfree(item);
> diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
> index 1498ef9724..39946f32d4 100644
> --- a/logprint/log_print_all.c
> +++ b/logprint/log_print_all.c
> @@ -78,7 +78,7 @@
> xfs_daddr_t blkno;
> struct xfs_disk_dquot *ddq;
>
> - f = (xfs_buf_log_format_t *)item->ri_buf[0].i_addr;
> + f = (xfs_buf_log_format_t *)item->ri_buf[0].iov_base;
> printf(" ");
> ASSERT(f->blf_type == XFS_LI_BUF);
> printf(_("BUF: #regs:%d start blkno:0x%llx len:%d bmap size:%d flags:0x%x\n"),
> @@ -87,8 +87,8 @@
> num = f->blf_size-1;
> i = 1;
> while (num-- > 0) {
> - p = item->ri_buf[i].i_addr;
> - len = item->ri_buf[i].i_len;
> + p = item->ri_buf[i].iov_base;
> + len = item->ri_buf[i].iov_len;
> i++;
> if (blkno == 0) { /* super block */
> struct xfs_dsb *dsb = (struct xfs_dsb *)p;
> @@ -185,7 +185,7 @@
> {
> xfs_qoff_logformat_t *qoff_f;
>
> - qoff_f = (xfs_qoff_logformat_t *)item->ri_buf[0].i_addr;
> + qoff_f = (xfs_qoff_logformat_t *)item->ri_buf[0].iov_base;
>
> ASSERT(qoff_f);
> printf(_("\tQUOTAOFF: #regs:%d type:"), qoff_f->qf_size);
> @@ -205,10 +205,10 @@
> xfs_dq_logformat_t *f;
> struct xfs_disk_dquot *d;
>
> - f = (xfs_dq_logformat_t *)item->ri_buf[0].i_addr;
> + f = (xfs_dq_logformat_t *)item->ri_buf[0].iov_base;
> ASSERT(f);
> ASSERT(f->qlf_len == 1);
> - d = (struct xfs_disk_dquot *)item->ri_buf[1].i_addr;
> + d = (struct xfs_disk_dquot *)item->ri_buf[1].iov_base;
> printf(_("\tDQUOT: #regs:%d blkno:%lld boffset:%u id: %d\n"),
> f->qlf_size, (long long)f->qlf_blkno, f->qlf_boffset, f->qlf_id);
> if (!print_quota)
> @@ -288,21 +288,22 @@
> int hasdata;
> int hasattr;
>
> - ASSERT(item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format_32) ||
> - item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format));
> - f = xfs_inode_item_format_convert(item->ri_buf[0].i_addr, item->ri_buf[0].i_len, &f_buf);
> + ASSERT(item->ri_buf[0].iov_len == sizeof(struct xfs_inode_log_format_32) ||
> + item->ri_buf[0].iov_len == sizeof(struct xfs_inode_log_format));
> + f = xfs_inode_item_format_convert(item->ri_buf[0].iov_base,
> + item->ri_buf[0].iov_len, &f_buf);
>
> printf(_(" INODE: #regs:%d ino:0x%llx flags:0x%x dsize:%d\n"),
> - f->ilf_size, (unsigned long long)f->ilf_ino, f->ilf_fields,
> - f->ilf_dsize);
> + f->ilf_size, (unsigned long long)f->ilf_ino, f->ilf_fields,
> + f->ilf_dsize);
>
> /* core inode comes 2nd */
> /* ASSERT len vs xfs_log_dinode_size() for V3 or V2 inodes */
> - ASSERT(item->ri_buf[1].i_len ==
> + ASSERT(item->ri_buf[1].iov_len ==
> offsetof(struct xfs_log_dinode, di_next_unlinked) ||
> - item->ri_buf[1].i_len == sizeof(struct xfs_log_dinode));
> + item->ri_buf[1].iov_len == sizeof(struct xfs_log_dinode));
> xlog_recover_print_inode_core((struct xfs_log_dinode *)
> - item->ri_buf[1].i_addr);
> + item->ri_buf[1].iov_base);
>
> hasdata = (f->ilf_fields & XFS_ILOG_DFORK) != 0;
> hasattr = (f->ilf_fields & XFS_ILOG_AFORK) != 0;
> @@ -312,22 +313,22 @@
> ASSERT(f->ilf_size == 3 + hasattr);
> printf(_(" DATA FORK EXTENTS inode data:\n"));
> if (print_inode && print_data)
> - xlog_recover_print_data(item->ri_buf[2].i_addr,
> - item->ri_buf[2].i_len);
> + xlog_recover_print_data(item->ri_buf[2].iov_base,
> + item->ri_buf[2].iov_len);
> break;
> case XFS_ILOG_DBROOT:
> ASSERT(f->ilf_size == 3 + hasattr);
> printf(_(" DATA FORK BTREE inode data:\n"));
> if (print_inode && print_data)
> - xlog_recover_print_data(item->ri_buf[2].i_addr,
> - item->ri_buf[2].i_len);
> + xlog_recover_print_data(item->ri_buf[2].iov_base,
> + item->ri_buf[2].iov_len);
> break;
> case XFS_ILOG_DDATA:
> ASSERT(f->ilf_size == 3 + hasattr);
> printf(_(" DATA FORK LOCAL inode data:\n"));
> if (print_inode && print_data)
> - xlog_recover_print_data(item->ri_buf[2].i_addr,
> - item->ri_buf[2].i_len);
> + xlog_recover_print_data(item->ri_buf[2].iov_base,
> + item->ri_buf[2].iov_len);
> break;
> case XFS_ILOG_DEV:
> ASSERT(f->ilf_size == 2 + hasattr);
> @@ -353,24 +354,24 @@
> printf(_(" ATTR FORK EXTENTS inode data:\n"));
> if (print_inode && print_data)
> xlog_recover_print_data(
> - item->ri_buf[attr_index].i_addr,
> - item->ri_buf[attr_index].i_len);
> + item->ri_buf[attr_index].iov_base,
> + item->ri_buf[attr_index].iov_len);
> break;
> case XFS_ILOG_ABROOT:
> ASSERT(f->ilf_size == 3 + hasdata);
> printf(_(" ATTR FORK BTREE inode data:\n"));
> if (print_inode && print_data)
> xlog_recover_print_data(
> - item->ri_buf[attr_index].i_addr,
> - item->ri_buf[attr_index].i_len);
> + item->ri_buf[attr_index].iov_base,
> + item->ri_buf[attr_index].iov_len);
> break;
> case XFS_ILOG_ADATA:
> ASSERT(f->ilf_size == 3 + hasdata);
> printf(_(" ATTR FORK LOCAL inode data:\n"));
> if (print_inode && print_data)
> xlog_recover_print_data(
> - item->ri_buf[attr_index].i_addr,
> - item->ri_buf[attr_index].i_len);
> + item->ri_buf[attr_index].iov_base,
> + item->ri_buf[attr_index].iov_len);
> break;
> default:
> xlog_panic("%s: illegal inode log flag", __FUNCTION__);
> @@ -385,7 +386,7 @@
> {
> struct xfs_icreate_log *icl;
>
> - icl = (struct xfs_icreate_log *)item->ri_buf[0].i_addr;
> + icl = (struct xfs_icreate_log *)item->ri_buf[0].iov_base;
>
> printf(_(" ICR: #ag: %d agbno: 0x%x len: %d\n"
> " cnt: %d isize: %d gen: 0x%x\n"),
> @@ -549,8 +550,8 @@
> */
> printf(_(": cnt:%d total:%d "), item->ri_cnt, item->ri_total);
> for (i=0; i<item->ri_cnt; i++) {
> - printf(_("a:0x%lx len:%d "),
> - (long)item->ri_buf[i].i_addr, item->ri_buf[i].i_len);
> + printf(_("a:0x%lx len:%zu "),
> + (long)item->ri_buf[i].iov_base, item->ri_buf[i].iov_len);
> }
> printf("\n");
> xlog_recover_print_logitem(item);
> diff --git a/logprint/log_redo.c b/logprint/log_redo.c
> index 89d7448342..f5bac21d35 100644
> --- a/logprint/log_redo.c
> +++ b/logprint/log_redo.c
> @@ -143,8 +143,8 @@
> int i;
> uint src_len, dst_len;
>
> - src_f = (xfs_efi_log_format_t *)item->ri_buf[0].i_addr;
> - src_len = item->ri_buf[0].i_len;
> + src_f = (xfs_efi_log_format_t *)item->ri_buf[0].iov_base;
> + src_len = item->ri_buf[0].iov_len;
> /*
> * An xfs_efi_log_format structure contains a variable length array
> * as the last field.
> @@ -229,7 +229,7 @@
> const char *item_name = "EFD?";
> xfs_efd_log_format_t *f;
>
> - f = (xfs_efd_log_format_t *)item->ri_buf[0].i_addr;
> + f = (xfs_efd_log_format_t *)item->ri_buf[0].iov_base;
>
> switch (f->efd_type) {
> case XFS_LI_EFD: item_name = "EFD"; break;
> @@ -355,8 +355,8 @@
> char *src_f;
> uint src_len;
>
> - src_f = item->ri_buf[0].i_addr;
> - src_len = item->ri_buf[0].i_len;
> + src_f = item->ri_buf[0].iov_base;
> + src_len = item->ri_buf[0].iov_len;
>
> xlog_print_trans_rui(&src_f, src_len, 0);
> }
> @@ -406,7 +406,7 @@
> {
> char *f;
>
> - f = item->ri_buf[0].i_addr;
> + f = item->ri_buf[0].iov_base;
> xlog_print_trans_rud(&f, sizeof(struct xfs_rud_log_format));
> }
>
> @@ -516,8 +516,8 @@
> char *src_f;
> uint src_len;
>
> - src_f = item->ri_buf[0].i_addr;
> - src_len = item->ri_buf[0].i_len;
> + src_f = item->ri_buf[0].iov_base;
> + src_len = item->ri_buf[0].iov_len;
>
> xlog_print_trans_cui(&src_f, src_len, 0);
> }
> @@ -563,7 +563,7 @@
> {
> char *f;
>
> - f = item->ri_buf[0].i_addr;
> + f = item->ri_buf[0].iov_base;
> xlog_print_trans_cud(&f, sizeof(struct xfs_cud_log_format));
> }
>
> @@ -667,8 +667,8 @@
> char *src_f;
> uint src_len;
>
> - src_f = item->ri_buf[0].i_addr;
> - src_len = item->ri_buf[0].i_len;
> + src_f = item->ri_buf[0].iov_base;
> + src_len = item->ri_buf[0].iov_len;
>
> xlog_print_trans_bui(&src_f, src_len, 0);
> }
> @@ -707,7 +707,7 @@
> {
> char *f;
>
> - f = item->ri_buf[0].i_addr;
> + f = item->ri_buf[0].iov_base;
> xlog_print_trans_bud(&f, sizeof(struct xfs_bud_log_format));
> }
>
> @@ -954,8 +954,8 @@
> unsigned int new_value_len = 0;
> int region = 0;
>
> - src_f = (struct xfs_attri_log_format *)item->ri_buf[0].i_addr;
> - src_len = item->ri_buf[region].i_len;
> + src_f = (struct xfs_attri_log_format *)item->ri_buf[0].iov_base;
> + src_len = item->ri_buf[region].iov_len;
>
> /*
> * An xfs_attri_log_format structure contains a attribute name and
> @@ -996,17 +996,17 @@
> if (name_len > 0) {
> region++;
> printf(_("ATTRI: name len:%u\n"), name_len);
> - print_or_dump((char *)item->ri_buf[region].i_addr,
> + print_or_dump((char *)item->ri_buf[region].iov_base,
> name_len);
> - name_ptr = item->ri_buf[region].i_addr;
> + name_ptr = item->ri_buf[region].iov_base;
> }
>
> if (new_name_len > 0) {
> region++;
> printf(_("ATTRI: newname len:%u\n"), new_name_len);
> - print_or_dump((char *)item->ri_buf[region].i_addr,
> + print_or_dump((char *)item->ri_buf[region].iov_base,
> new_name_len);
> - new_name_ptr = item->ri_buf[region].i_addr;
> + new_name_ptr = item->ri_buf[region].iov_base;
> }
>
> if (value_len > 0) {
> @@ -1014,8 +1014,8 @@
>
> region++;
> printf(_("ATTRI: value len:%u\n"), value_len);
> - print_or_dump((char *)item->ri_buf[region].i_addr, len);
> - value_ptr = item->ri_buf[region].i_addr;
> + print_or_dump((char *)item->ri_buf[region].iov_base, len);
> + value_ptr = item->ri_buf[region].iov_base;
> }
>
> if (new_value_len > 0) {
> @@ -1023,8 +1023,8 @@
>
> region++;
> printf(_("ATTRI: newvalue len:%u\n"), new_value_len);
> - print_or_dump((char *)item->ri_buf[region].i_addr, len);
> - new_value_ptr = item->ri_buf[region].i_addr;
> + print_or_dump((char *)item->ri_buf[region].iov_base, len);
> + new_value_ptr = item->ri_buf[region].iov_base;
> }
>
> if (src_f->alfi_attr_filter & XFS_ATTR_PARENT)
> @@ -1065,7 +1065,7 @@
> {
> struct xfs_attrd_log_format *f;
>
> - f = (struct xfs_attrd_log_format *)item->ri_buf[0].i_addr;
> + f = (struct xfs_attrd_log_format *)item->ri_buf[0].iov_base;
>
> printf(_(" ATTRD: #regs: %d id: 0x%llx\n"),
> f->alfd_size,
> @@ -1156,8 +1156,8 @@
> char *src_f;
> uint src_len;
>
> - src_f = item->ri_buf[0].i_addr;
> - src_len = item->ri_buf[0].i_len;
> + src_f = item->ri_buf[0].iov_base;
> + src_len = item->ri_buf[0].iov_len;
>
> xlog_print_trans_xmi(&src_f, src_len, 0);
> }
> @@ -1196,6 +1196,6 @@
> {
> char *f;
>
> - f = item->ri_buf[0].i_addr;
> + f = item->ri_buf[0].iov_base;
> xlog_print_trans_xmd(&f, sizeof(struct xfs_xmd_log_format));
> }
> diff --git a/repair/phase2.c b/repair/phase2.c
> index e249980527..fc96f9c422 100644
> --- a/repair/phase2.c
> +++ b/repair/phase2.c
> @@ -296,11 +296,7 @@
> * there while we try to make a per-AG reservation with the new
> * geometry.
> */
> - error = -libxfs_trans_alloc_empty(mp, &tp);
> - if (error)
> - do_error(
> - _("Cannot reserve resources for upgrade check, err=%d.\n"),
> - error);
> + tp = libxfs_trans_alloc_empty(mp);
>
> error = -libxfs_ialloc_read_agi(pag, tp, 0, &agi_bp);
> if (error)
> diff --git a/repair/pptr.c b/repair/pptr.c
> index ac0a9c618b..6a9e072b36 100644
> --- a/repair/pptr.c
> +++ b/repair/pptr.c
> @@ -1217,7 +1217,7 @@
> fscan->have_garbage = false;
> fscan->nr_file_pptrs = 0;
>
> - libxfs_trans_alloc_empty(ip->i_mount, &tp);
> + tp = libxfs_trans_alloc_empty(ip->i_mount);
> error = xattr_walk(tp, ip, examine_xattr, fscan);
> if (tp)
> libxfs_trans_cancel(tp);
> @@ -1417,7 +1417,7 @@
> do_error("init garbage pptr names failed: %s\n",
> strerror(error));
>
> - libxfs_trans_alloc_empty(ip->i_mount, &tp);
> + tp = libxfs_trans_alloc_empty(ip->i_mount);
> error = xattr_walk(tp, ip, erase_pptrs, &fscan);
> if (tp)
> libxfs_trans_cancel(tp);
> diff --git a/repair/quotacheck.c b/repair/quotacheck.c
> index df6cde2d58..f4c0314177 100644
> --- a/repair/quotacheck.c
> +++ b/repair/quotacheck.c
> @@ -437,9 +437,7 @@
> if (!dquots || !chkd_flags)
> return;
>
> - error = -libxfs_trans_alloc_empty(mp, &tp);
> - if (error)
> - do_error(_("could not alloc transaction to open quota file\n"));
> + tp = libxfs_trans_alloc_empty(mp);
>
> ino = get_quota_inode(type);
> error = -libxfs_trans_metafile_iget(tp, ino, metafile_type, &ip);
> @@ -679,9 +677,7 @@
> struct xfs_inode *dp = NULL;
> int error, err2;
>
> - error = -libxfs_trans_alloc_empty(mp, &tp);
> - if (error)
> - goto out;
> + tp = libxfs_trans_alloc_empty(mp);
>
> error = -libxfs_dqinode_load_parent(tp, &dp);
> if (error)
> @@ -698,7 +694,6 @@
> libxfs_irele(dp);
> out_cancel:
> libxfs_trans_cancel(tp);
> -out:
> if (error) {
> switch (error) {
> case EFSCORRUPTED:
> diff --git a/repair/rcbag.c b/repair/rcbag.c
> index 21732b65c6..d7addbf58e 100644
> --- a/repair/rcbag.c
> +++ b/repair/rcbag.c
> @@ -95,9 +95,7 @@
> int has;
> int error;
>
> - error = -libxfs_trans_alloc_empty(mp, &tp);
> - if (error)
> - do_error(_("allocating tx for refcount bag update\n"));
> + tp = libxfs_trans_alloc_empty(mp);
>
> cur = rcbagbt_mem_cursor(mp, tp, &bag->xfbtree);
> error = rcbagbt_lookup_eq(cur, rmap, &has);
> @@ -217,9 +215,7 @@
> int has;
> int error;
>
> - error = -libxfs_trans_alloc_empty(mp, &tp);
> - if (error)
> - do_error(_("allocating tx for refcount bag update\n"));
> + tp = libxfs_trans_alloc_empty(mp);
>
> /* go to the right edge of the tree */
> cur = rcbagbt_mem_cursor(mp, tp, &bag->xfbtree);
> diff --git a/repair/rcbag_btree.c b/repair/rcbag_btree.c
> index bed7c7a8f6..fc5f69c4d2 100644
> --- a/repair/rcbag_btree.c
> +++ b/repair/rcbag_btree.c
> @@ -46,8 +46,8 @@
> bag_rec->rbg_refcount = bag_irec->rbg_refcount;
> }
>
> -STATIC int64_t
> -rcbagbt_key_diff(
> +STATIC int
> +rcbagbt_cmp_key_with_cur(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *key)
> {
> @@ -72,8 +72,8 @@
> return 0;
> }
>
> -STATIC int64_t
> -rcbagbt_diff_two_keys(
> +STATIC int
> +rcbagbt_cmp_two_keys(
> struct xfs_btree_cur *cur,
> const union xfs_btree_key *k1,
> const union xfs_btree_key *k2,
> @@ -220,9 +220,9 @@
> .init_key_from_rec = rcbagbt_init_key_from_rec,
> .init_rec_from_cur = rcbagbt_init_rec_from_cur,
> .init_ptr_from_cur = xfbtree_init_ptr_from_cur,
> - .key_diff = rcbagbt_key_diff,
> + .cmp_key_with_cur = rcbagbt_cmp_key_with_cur,
> .buf_ops = &rcbagbt_mem_buf_ops,
> - .diff_two_keys = rcbagbt_diff_two_keys,
> + .cmp_two_keys = rcbagbt_cmp_two_keys,
> .keys_inorder = rcbagbt_keys_inorder,
> .recs_inorder = rcbagbt_recs_inorder,
> };
> diff --git a/repair/rmap.c b/repair/rmap.c
> index 97510dd875..e89bd32d63 100644
> --- a/repair/rmap.c
> +++ b/repair/rmap.c
> @@ -323,9 +323,7 @@
> int error;
>
> xfbt = &rmaps_for_group(isrt, agno)->ar_xfbtree;
> - error = -libxfs_trans_alloc_empty(mp, &tp);
> - if (error)
> - do_error(_("allocating tx for in-memory rmap update\n"));
> + tp = libxfs_trans_alloc_empty(mp);
>
> error = rmap_init_mem_cursor(mp, tp, isrt, agno, &rmcur);
> if (error)
> diff --git a/repair/rt.c b/repair/rt.c
> index 1ac2bf6fc4..781d896844 100644
> --- a/repair/rt.c
> +++ b/repair/rt.c
> @@ -301,10 +301,7 @@
> if (rtg->rtg_inodes[type])
> goto out_rtg;
>
> - error = -libxfs_trans_alloc_empty(mp, &tp);
> - if (error)
> - goto out_rtg;
> -
> + tp = libxfs_trans_alloc_empty(mp);
>
> error = -libxfs_rtginode_load(rtg, type, tp);
> if (error)
> @@ -497,9 +494,7 @@
> int error, err2;
> int i;
>
> - error = -libxfs_trans_alloc_empty(mp, &tp);
> - if (error)
> - goto out;
> + tp = libxfs_trans_alloc_empty(mp);
> if (xfs_has_rtgroups(mp) && mp->m_sb.sb_rgcount > 0) {
> error = -libxfs_rtginode_load_parent(tp);
> if (error)
> @@ -516,7 +511,6 @@
>
> out_cancel:
> libxfs_trans_cancel(tp);
> -out:
> if (xfs_has_rtgroups(mp) && error) {
> /*
> * Old xfs_repair didn't complain if rtbitmaps didn't load
> diff --git a/scrub/inodes.c b/scrub/inodes.c
> index 2f3c87be79..4ed7cd9963 100644
> --- a/scrub/inodes.c
> +++ b/scrub/inodes.c
> @@ -197,8 +197,6 @@
> return seen_mask;
> }
>
> -#define cmp_int(l, r) ((l > r) - (l < r))
> -
> /* Compare two bulkstat records by inumber. */
> static int
> compare_bstat(
^ permalink raw reply [flat|nested] 15+ messages in thread