From: "Darrick J. Wong" <djwong@kernel.org>
To: Fedor Pchelkin <pchelkin@ispras.ru>
Cc: linux-xfs@vger.kernel.org, aalbersh@kernel.org, cem@kernel.org,
cmaiolino@redhat.com, hch@lst.de, pranav.tyagi03@gmail.com,
sandeen@redhat.com
Subject: Re: [PATCH 4/11] [PATCH] xfs: refactor cmp_key_with_cur routines to take advantage of cmp_int()
Date: Wed, 8 Oct 2025 13:36:33 -0700 [thread overview]
Message-ID: <20251008203633.GB6215@frogsfrogsfrogs> (raw)
In-Reply-To: <vzli4wtrpsrkzklbm6c4afm3h6s4izso6yscruif46covebhn4@l6v5uftifs4l>
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
next prev parent reply other threads:[~2025-10-08 20:36 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` [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
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 [this message]
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 ` [PATCH 6/11] [PATCH] xfs: refactor xfs_btree_diff_two_ptrs() to take advantage of cmp_int() Fedor Pchelkin
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 ` [PATCH 8/11] [PATCH] xfs: improve the xg_active_ref check in xfs_group_free Christoph Hellwig
2025-10-08 16:56 ` [PATCH 9/11] [PATCH] fs/xfs: replace strncpy with memtostr_pad() 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
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251008203633.GB6215@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=aalbersh@kernel.org \
--cc=cem@kernel.org \
--cc=cmaiolino@redhat.com \
--cc=hch@lst.de \
--cc=linux-xfs@vger.kernel.org \
--cc=pchelkin@ispras.ru \
--cc=pranav.tyagi03@gmail.com \
--cc=sandeen@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox