* [PATCH 0/3] xfs: minor cleanups
@ 2018-06-07 5:27 Dave Chinner
2018-06-07 5:27 ` [PATCH 1/3] xfs: move various type verifiers to common file Dave Chinner
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Dave Chinner @ 2018-06-07 5:27 UTC (permalink / raw)
To: linux-xfs
Hi folks,
These patches are minor cleanups that do not change the behaviour
of the code. The first two replace abstractions
that have outlived their usefulness with linux kernel native
primitives. It's been tested on both 32 and 64 bit platforms.
The last patch moves all the new(-ish) type verifiers to a common
file in libxfs - this means they are easy to find and we have a
central place for adding future type verifiers.
Cheers,
Dave.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] xfs: move various type verifiers to common file
2018-06-07 5:27 [PATCH 0/3] xfs: minor cleanups Dave Chinner
@ 2018-06-07 5:27 ` Dave Chinner
2018-06-07 11:41 ` Brian Foster
` (2 more replies)
2018-06-07 5:27 ` [PATCH 2/3] xfs: replace do_mod with native operations Dave Chinner
2018-06-07 5:27 ` [PATCH 3/3] xfs: clean up MIN/MAX Dave Chinner
2 siblings, 3 replies; 17+ messages in thread
From: Dave Chinner @ 2018-06-07 5:27 UTC (permalink / raw)
To: linux-xfs
From: Dave Chinner <dchinner@redhat.com>
New verification functions like xfs_verify_fsbno() and
xfs_verify_agino() are spread across multiple files and different
header files. They really don't fit cleanly into the places they've
been put, and have wider scope than the current header includes.
Move the type verifiers to a new file in libxfs (xfs-types.c) and
the prototypes to xfs_types.h where they will be visible to all the
code that uses the types.
Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/Makefile | 1 +
fs/xfs/libxfs/xfs_alloc.c | 49 ----------
fs/xfs/libxfs/xfs_alloc.h | 4 -
fs/xfs/libxfs/xfs_ialloc.c | 90 ------------------
fs/xfs/libxfs/xfs_ialloc.h | 7 --
fs/xfs/libxfs/xfs_rtbitmap.c | 12 ---
fs/xfs/libxfs/xfs_types.c | 173 +++++++++++++++++++++++++++++++++++
fs/xfs/libxfs/xfs_types.h | 19 ++++
| 2 +-
9 files changed, 194 insertions(+), 163 deletions(-)
create mode 100644 fs/xfs/libxfs/xfs_types.c
diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
index 19a1f8064d8a..5333f7cbc95a 100644
--- a/fs/xfs/Makefile
+++ b/fs/xfs/Makefile
@@ -50,6 +50,7 @@ xfs-y += $(addprefix libxfs/, \
xfs_sb.o \
xfs_symlink_remote.o \
xfs_trans_resv.o \
+ xfs_types.o \
)
# xfs_rtbitmap is shared with libxfs
xfs-$(CONFIG_XFS_RT) += $(addprefix libxfs/, \
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 1db50cfc0212..eef466260d43 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -3123,55 +3123,6 @@ xfs_alloc_query_all(
return xfs_btree_query_all(cur, xfs_alloc_query_range_helper, &query);
}
-/* Find the size of the AG, in blocks. */
-xfs_agblock_t
-xfs_ag_block_count(
- struct xfs_mount *mp,
- xfs_agnumber_t agno)
-{
- ASSERT(agno < mp->m_sb.sb_agcount);
-
- if (agno < mp->m_sb.sb_agcount - 1)
- return mp->m_sb.sb_agblocks;
- return mp->m_sb.sb_dblocks - (agno * mp->m_sb.sb_agblocks);
-}
-
-/*
- * Verify that an AG block number pointer neither points outside the AG
- * nor points at static metadata.
- */
-bool
-xfs_verify_agbno(
- struct xfs_mount *mp,
- xfs_agnumber_t agno,
- xfs_agblock_t agbno)
-{
- xfs_agblock_t eoag;
-
- eoag = xfs_ag_block_count(mp, agno);
- if (agbno >= eoag)
- return false;
- if (agbno <= XFS_AGFL_BLOCK(mp))
- return false;
- return true;
-}
-
-/*
- * Verify that an FS block number pointer neither points outside the
- * filesystem nor points at static AG metadata.
- */
-bool
-xfs_verify_fsbno(
- struct xfs_mount *mp,
- xfs_fsblock_t fsbno)
-{
- xfs_agnumber_t agno = XFS_FSB_TO_AGNO(mp, fsbno);
-
- if (agno >= mp->m_sb.sb_agcount)
- return false;
- return xfs_verify_agbno(mp, agno, XFS_FSB_TO_AGBNO(mp, fsbno));
-}
-
/* Is there a record covering a given extent? */
int
xfs_alloc_has_record(
diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
index 1651d924aaf1..e716c993ac4c 100644
--- a/fs/xfs/libxfs/xfs_alloc.h
+++ b/fs/xfs/libxfs/xfs_alloc.h
@@ -242,10 +242,6 @@ int xfs_alloc_query_range(struct xfs_btree_cur *cur,
xfs_alloc_query_range_fn fn, void *priv);
int xfs_alloc_query_all(struct xfs_btree_cur *cur, xfs_alloc_query_range_fn fn,
void *priv);
-xfs_agblock_t xfs_ag_block_count(struct xfs_mount *mp, xfs_agnumber_t agno);
-bool xfs_verify_agbno(struct xfs_mount *mp, xfs_agnumber_t agno,
- xfs_agblock_t agbno);
-bool xfs_verify_fsbno(struct xfs_mount *mp, xfs_fsblock_t fsbno);
int xfs_alloc_has_record(struct xfs_btree_cur *cur, xfs_agblock_t bno,
xfs_extlen_t len, bool *exist);
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 3f551eb29157..8ec39dad62d7 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -2674,96 +2674,6 @@ xfs_ialloc_pagi_init(
return 0;
}
-/* Calculate the first and last possible inode number in an AG. */
-void
-xfs_ialloc_agino_range(
- struct xfs_mount *mp,
- xfs_agnumber_t agno,
- xfs_agino_t *first,
- xfs_agino_t *last)
-{
- xfs_agblock_t bno;
- xfs_agblock_t eoag;
-
- eoag = xfs_ag_block_count(mp, agno);
-
- /*
- * Calculate the first inode, which will be in the first
- * cluster-aligned block after the AGFL.
- */
- bno = round_up(XFS_AGFL_BLOCK(mp) + 1,
- xfs_ialloc_cluster_alignment(mp));
- *first = XFS_OFFBNO_TO_AGINO(mp, bno, 0);
-
- /*
- * Calculate the last inode, which will be at the end of the
- * last (aligned) cluster that can be allocated in the AG.
- */
- bno = round_down(eoag, xfs_ialloc_cluster_alignment(mp));
- *last = XFS_OFFBNO_TO_AGINO(mp, bno, 0) - 1;
-}
-
-/*
- * Verify that an AG inode number pointer neither points outside the AG
- * nor points at static metadata.
- */
-bool
-xfs_verify_agino(
- struct xfs_mount *mp,
- xfs_agnumber_t agno,
- xfs_agino_t agino)
-{
- xfs_agino_t first;
- xfs_agino_t last;
-
- xfs_ialloc_agino_range(mp, agno, &first, &last);
- return agino >= first && agino <= last;
-}
-
-/*
- * Verify that an FS inode number pointer neither points outside the
- * filesystem nor points at static AG metadata.
- */
-bool
-xfs_verify_ino(
- struct xfs_mount *mp,
- xfs_ino_t ino)
-{
- xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, ino);
- xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ino);
-
- if (agno >= mp->m_sb.sb_agcount)
- return false;
- if (XFS_AGINO_TO_INO(mp, agno, agino) != ino)
- return false;
- return xfs_verify_agino(mp, agno, agino);
-}
-
-/* Is this an internal inode number? */
-bool
-xfs_internal_inum(
- struct xfs_mount *mp,
- xfs_ino_t ino)
-{
- return ino == mp->m_sb.sb_rbmino || ino == mp->m_sb.sb_rsumino ||
- (xfs_sb_version_hasquota(&mp->m_sb) &&
- xfs_is_quota_inode(&mp->m_sb, ino));
-}
-
-/*
- * Verify that a directory entry's inode number doesn't point at an internal
- * inode, empty space, or static AG metadata.
- */
-bool
-xfs_verify_dir_ino(
- struct xfs_mount *mp,
- xfs_ino_t ino)
-{
- if (xfs_internal_inum(mp, ino))
- return false;
- return xfs_verify_ino(mp, ino);
-}
-
/* Is there an inode record covering a given range of inode numbers? */
int
xfs_ialloc_has_inode_record(
diff --git a/fs/xfs/libxfs/xfs_ialloc.h b/fs/xfs/libxfs/xfs_ialloc.h
index 144f3eac9b93..90b09c5f163b 100644
--- a/fs/xfs/libxfs/xfs_ialloc.h
+++ b/fs/xfs/libxfs/xfs_ialloc.h
@@ -169,12 +169,5 @@ int xfs_inobt_insert_rec(struct xfs_btree_cur *cur, uint16_t holemask,
int *stat);
int xfs_ialloc_cluster_alignment(struct xfs_mount *mp);
-void xfs_ialloc_agino_range(struct xfs_mount *mp, xfs_agnumber_t agno,
- xfs_agino_t *first, xfs_agino_t *last);
-bool xfs_verify_agino(struct xfs_mount *mp, xfs_agnumber_t agno,
- xfs_agino_t agino);
-bool xfs_verify_ino(struct xfs_mount *mp, xfs_ino_t ino);
-bool xfs_internal_inum(struct xfs_mount *mp, xfs_ino_t ino);
-bool xfs_verify_dir_ino(struct xfs_mount *mp, xfs_ino_t ino);
#endif /* __XFS_IALLOC_H__ */
diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
index ffc72075a44e..65fc4ed2e9a1 100644
--- a/fs/xfs/libxfs/xfs_rtbitmap.c
+++ b/fs/xfs/libxfs/xfs_rtbitmap.c
@@ -1080,18 +1080,6 @@ xfs_rtalloc_query_all(
return xfs_rtalloc_query_range(tp, &keys[0], &keys[1], fn, priv);
}
-/*
- * Verify that an realtime block number pointer doesn't point off the
- * end of the realtime device.
- */
-bool
-xfs_verify_rtbno(
- struct xfs_mount *mp,
- xfs_rtblock_t rtbno)
-{
- return rtbno < mp->m_sb.sb_rblocks;
-}
-
/* Is the given extent all free? */
int
xfs_rtalloc_extent_is_free(
diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
new file mode 100644
index 000000000000..2e2a243cef2e
--- /dev/null
+++ b/fs/xfs/libxfs/xfs_types.c
@@ -0,0 +1,173 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2000-2002,2005 Silicon Graphics, Inc.
+ * Copyright (C) 2017 Oracle.
+ * All Rights Reserved.
+ */
+#include "xfs.h"
+#include "xfs_fs.h"
+#include "xfs_format.h"
+#include "xfs_log_format.h"
+#include "xfs_shared.h"
+#include "xfs_trans_resv.h"
+#include "xfs_bit.h"
+#include "xfs_sb.h"
+#include "xfs_mount.h"
+#include "xfs_defer.h"
+#include "xfs_inode.h"
+#include "xfs_btree.h"
+#include "xfs_rmap.h"
+#include "xfs_alloc_btree.h"
+#include "xfs_alloc.h"
+#include "xfs_ialloc.h"
+
+/* Find the size of the AG, in blocks. */
+xfs_agblock_t
+xfs_ag_block_count(
+ struct xfs_mount *mp,
+ xfs_agnumber_t agno)
+{
+ ASSERT(agno < mp->m_sb.sb_agcount);
+
+ if (agno < mp->m_sb.sb_agcount - 1)
+ return mp->m_sb.sb_agblocks;
+ return mp->m_sb.sb_dblocks - (agno * mp->m_sb.sb_agblocks);
+}
+
+/*
+ * Verify that an AG block number pointer neither points outside the AG
+ * nor points at static metadata.
+ */
+bool
+xfs_verify_agbno(
+ struct xfs_mount *mp,
+ xfs_agnumber_t agno,
+ xfs_agblock_t agbno)
+{
+ xfs_agblock_t eoag;
+
+ eoag = xfs_ag_block_count(mp, agno);
+ if (agbno >= eoag)
+ return false;
+ if (agbno <= XFS_AGFL_BLOCK(mp))
+ return false;
+ return true;
+}
+
+/*
+ * Verify that an FS block number pointer neither points outside the
+ * filesystem nor points at static AG metadata.
+ */
+bool
+xfs_verify_fsbno(
+ struct xfs_mount *mp,
+ xfs_fsblock_t fsbno)
+{
+ xfs_agnumber_t agno = XFS_FSB_TO_AGNO(mp, fsbno);
+
+ if (agno >= mp->m_sb.sb_agcount)
+ return false;
+ return xfs_verify_agbno(mp, agno, XFS_FSB_TO_AGBNO(mp, fsbno));
+}
+
+/* Calculate the first and last possible inode number in an AG. */
+void
+xfs_agino_range(
+ struct xfs_mount *mp,
+ xfs_agnumber_t agno,
+ xfs_agino_t *first,
+ xfs_agino_t *last)
+{
+ xfs_agblock_t bno;
+ xfs_agblock_t eoag;
+
+ eoag = xfs_ag_block_count(mp, agno);
+
+ /*
+ * Calculate the first inode, which will be in the first
+ * cluster-aligned block after the AGFL.
+ */
+ bno = round_up(XFS_AGFL_BLOCK(mp) + 1,
+ xfs_ialloc_cluster_alignment(mp));
+ *first = XFS_OFFBNO_TO_AGINO(mp, bno, 0);
+
+ /*
+ * Calculate the last inode, which will be at the end of the
+ * last (aligned) cluster that can be allocated in the AG.
+ */
+ bno = round_down(eoag, xfs_ialloc_cluster_alignment(mp));
+ *last = XFS_OFFBNO_TO_AGINO(mp, bno, 0) - 1;
+}
+
+/*
+ * Verify that an AG inode number pointer neither points outside the AG
+ * nor points at static metadata.
+ */
+bool
+xfs_verify_agino(
+ struct xfs_mount *mp,
+ xfs_agnumber_t agno,
+ xfs_agino_t agino)
+{
+ xfs_agino_t first;
+ xfs_agino_t last;
+
+ xfs_agino_range(mp, agno, &first, &last);
+ return agino >= first && agino <= last;
+}
+
+/*
+ * Verify that an FS inode number pointer neither points outside the
+ * filesystem nor points at static AG metadata.
+ */
+bool
+xfs_verify_ino(
+ struct xfs_mount *mp,
+ xfs_ino_t ino)
+{
+ xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, ino);
+ xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ino);
+
+ if (agno >= mp->m_sb.sb_agcount)
+ return false;
+ if (XFS_AGINO_TO_INO(mp, agno, agino) != ino)
+ return false;
+ return xfs_verify_agino(mp, agno, agino);
+}
+
+/* Is this an internal inode number? */
+bool
+xfs_internal_inum(
+ struct xfs_mount *mp,
+ xfs_ino_t ino)
+{
+ return ino == mp->m_sb.sb_rbmino || ino == mp->m_sb.sb_rsumino ||
+ (xfs_sb_version_hasquota(&mp->m_sb) &&
+ xfs_is_quota_inode(&mp->m_sb, ino));
+}
+
+/*
+ * Verify that a directory entry's inode number doesn't point at an internal
+ * inode, empty space, or static AG metadata.
+ */
+bool
+xfs_verify_dir_ino(
+ struct xfs_mount *mp,
+ xfs_ino_t ino)
+{
+ if (xfs_internal_inum(mp, ino))
+ return false;
+ return xfs_verify_ino(mp, ino);
+}
+
+/*
+ * Verify that an realtime block number pointer doesn't point off the
+ * end of the realtime device.
+ */
+bool
+xfs_verify_rtbno(
+ struct xfs_mount *mp,
+ xfs_rtblock_t rtbno)
+{
+ return rtbno < mp->m_sb.sb_rblocks;
+}
diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
index b72ae343140e..4055d62f690c 100644
--- a/fs/xfs/libxfs/xfs_types.h
+++ b/fs/xfs/libxfs/xfs_types.h
@@ -147,4 +147,23 @@ typedef struct xfs_bmbt_irec
xfs_exntst_t br_state; /* extent state */
} xfs_bmbt_irec_t;
+/*
+ * Type verifier functions
+ */
+struct xfs_mount;
+
+xfs_agblock_t xfs_ag_block_count(struct xfs_mount *mp, xfs_agnumber_t agno);
+bool xfs_verify_agbno(struct xfs_mount *mp, xfs_agnumber_t agno,
+ xfs_agblock_t agbno);
+bool xfs_verify_fsbno(struct xfs_mount *mp, xfs_fsblock_t fsbno);
+
+void xfs_agino_range(struct xfs_mount *mp, xfs_agnumber_t agno,
+ xfs_agino_t *first, xfs_agino_t *last);
+bool xfs_verify_agino(struct xfs_mount *mp, xfs_agnumber_t agno,
+ xfs_agino_t agino);
+bool xfs_verify_ino(struct xfs_mount *mp, xfs_ino_t ino);
+bool xfs_internal_inum(struct xfs_mount *mp, xfs_ino_t ino);
+bool xfs_verify_dir_ino(struct xfs_mount *mp, xfs_ino_t ino);
+bool xfs_verify_rtbno(struct xfs_mount *mp, xfs_rtblock_t rtbno);
+
#endif /* __XFS_TYPES_H__ */
--git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index fb9637ff4bde..9bb0745f1ad2 100644
--- a/fs/xfs/scrub/agheader.c
+++ b/fs/xfs/scrub/agheader.c
@@ -867,7 +867,7 @@ xfs_scrub_agi(
}
/* Check inode counters */
- xfs_ialloc_agino_range(mp, agno, &first_agino, &last_agino);
+ xfs_agino_range(mp, agno, &first_agino, &last_agino);
icount = be32_to_cpu(agi->agi_count);
if (icount > last_agino - first_agino + 1 ||
icount < be32_to_cpu(agi->agi_freecount))
--
2.17.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/3] xfs: replace do_mod with native operations
2018-06-07 5:27 [PATCH 0/3] xfs: minor cleanups Dave Chinner
2018-06-07 5:27 ` [PATCH 1/3] xfs: move various type verifiers to common file Dave Chinner
@ 2018-06-07 5:27 ` Dave Chinner
2018-06-07 11:42 ` Brian Foster
` (2 more replies)
2018-06-07 5:27 ` [PATCH 3/3] xfs: clean up MIN/MAX Dave Chinner
2 siblings, 3 replies; 17+ messages in thread
From: Dave Chinner @ 2018-06-07 5:27 UTC (permalink / raw)
To: linux-xfs
From: Dave Chinner <dchinner@redhat.com>
do_mod() is a hold-over from when we have different sizes for file
offsets and and other internal values for 40 bit XFS filesystems.
Hence depending on build flags variables passed to do_mod() could
change size. We no longer support those small format filesystems and
hence everything is of fixed size theses days, even on 32 bit
platforms.
As such, we can convert all the do_mod() callers to platform
optimised modulus operations as defined by linux/math64.h.
Individual conversions depend on the types of variables being used.
Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 37 +++++++++++++++++++++++--------------
fs/xfs/xfs_bmap_util.c | 14 +++++++++-----
fs/xfs/xfs_inode.c | 2 +-
fs/xfs/xfs_iomap.h | 4 ++--
fs/xfs/xfs_linux.h | 19 -------------------
fs/xfs/xfs_log_recover.c | 32 +++++++++++++++++++++++++-------
fs/xfs/xfs_rtalloc.c | 15 +++++++++++----
7 files changed, 71 insertions(+), 52 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 3de047eb8209..4d8fd37ec7ae 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -2923,7 +2923,7 @@ xfs_bmap_extsize_align(
* perform this alignment, or if a truncate shot us in the
* foot.
*/
- temp = do_mod(orig_off, extsz);
+ div_u64_rem(orig_off, extsz, &temp);
if (temp) {
align_alen += temp;
align_off -= temp;
@@ -3497,15 +3497,17 @@ xfs_bmap_btalloc(
/* apply extent size hints if obtained earlier */
if (align) {
args.prod = align;
- if ((args.mod = (xfs_extlen_t)do_mod(ap->offset, args.prod)))
- args.mod = (xfs_extlen_t)(args.prod - args.mod);
+ div_u64_rem(ap->offset, args.prod, &args.mod);
+ if (args.mod)
+ args.mod = args.prod - args.mod;
} else if (mp->m_sb.sb_blocksize >= PAGE_SIZE) {
args.prod = 1;
args.mod = 0;
} else {
args.prod = PAGE_SIZE >> mp->m_sb.sb_blocklog;
- if ((args.mod = (xfs_extlen_t)(do_mod(ap->offset, args.prod))))
- args.mod = (xfs_extlen_t)(args.prod - args.mod);
+ div_u64_rem(ap->offset, args.prod, &args.mod);
+ if (args.mod)
+ args.mod = args.prod - args.mod;
}
/*
* If we are not low on available data blocks, and the
@@ -4953,13 +4955,15 @@ xfs_bmap_del_extent_real(
if (whichfork == XFS_DATA_FORK && XFS_IS_REALTIME_INODE(ip)) {
xfs_fsblock_t bno;
xfs_filblks_t len;
+ xfs_extlen_t mod;
+
+ bno = div_u64_rem(del->br_startblock, mp->m_sb.sb_rextsize,
+ &mod);
+ ASSERT(mod == 0);
+ len = div_u64_rem(del->br_blockcount, mp->m_sb.sb_rextsize,
+ &mod);
+ ASSERT(mod == 0);
- ASSERT(do_mod(del->br_blockcount, mp->m_sb.sb_rextsize) == 0);
- ASSERT(do_mod(del->br_startblock, mp->m_sb.sb_rextsize) == 0);
- bno = del->br_startblock;
- len = del->br_blockcount;
- do_div(bno, mp->m_sb.sb_rextsize);
- do_div(len, mp->m_sb.sb_rextsize);
error = xfs_rtfree_extent(tp, bno, (xfs_extlen_t)len);
if (error)
goto done;
@@ -5296,9 +5300,12 @@ __xfs_bunmapi(
del.br_blockcount = max_len;
}
+ if (!isrt)
+ goto delete;
+
sum = del.br_startblock + del.br_blockcount;
- if (isrt &&
- (mod = do_mod(sum, mp->m_sb.sb_rextsize))) {
+ div_u64_rem(sum, mp->m_sb.sb_rextsize, &mod);
+ if (mod) {
/*
* Realtime extent not lined up at the end.
* The extent could have been split into written
@@ -5345,7 +5352,8 @@ __xfs_bunmapi(
goto error0;
goto nodelete;
}
- if (isrt && (mod = do_mod(del.br_startblock, mp->m_sb.sb_rextsize))) {
+ div_u64_rem(del.br_startblock, mp->m_sb.sb_rextsize, &mod);
+ if (mod) {
/*
* Realtime extent is lined up at the end but not
* at the front. We'll get rid of full extents if
@@ -5414,6 +5422,7 @@ __xfs_bunmapi(
}
}
+delete:
if (wasdel) {
error = xfs_bmap_del_extent_delay(ip, whichfork, &icur,
&got, &del);
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 7d26933a542f..13e6caf8b801 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -80,6 +80,7 @@ xfs_bmap_rtalloc(
int error; /* error return value */
xfs_mount_t *mp; /* mount point structure */
xfs_extlen_t prod = 0; /* product factor for allocators */
+ xfs_extlen_t mod = 0; /* product factor for allocators */
xfs_extlen_t ralen = 0; /* realtime allocation length */
xfs_extlen_t align; /* minimum allocation alignment */
xfs_rtblock_t rtb;
@@ -99,7 +100,8 @@ xfs_bmap_rtalloc(
* If the offset & length are not perfectly aligned
* then kill prod, it will just get us in trouble.
*/
- if (do_mod(ap->offset, align) || ap->length % align)
+ div_u64_rem(ap->offset, align, &mod);
+ if (mod || ap->length % align)
prod = 1;
/*
* Set ralen to be the actual requested length in rtextents.
@@ -936,9 +938,11 @@ xfs_alloc_file_space(
do_div(s, extsz);
s *= extsz;
e = startoffset_fsb + allocatesize_fsb;
- if ((temp = do_mod(startoffset_fsb, extsz)))
+ div_u64_rem(startoffset_fsb, extsz, &temp);
+ if (temp)
e += temp;
- if ((temp = do_mod(e, extsz)))
+ div_u64_rem(e, extsz, &temp);
+ if (temp)
e += extsz - temp;
} else {
s = 0;
@@ -1090,7 +1094,7 @@ xfs_adjust_extent_unmap_boundaries(
struct xfs_mount *mp = ip->i_mount;
struct xfs_bmbt_irec imap;
int nimap, error;
- xfs_extlen_t mod = 0;
+ xfs_rtblock_t mod = 0;
nimap = 1;
error = xfs_bmapi_read(ip, *startoffset_fsb, 1, &imap, &nimap, 0);
@@ -1099,7 +1103,7 @@ xfs_adjust_extent_unmap_boundaries(
if (nimap && imap.br_startblock != HOLESTARTBLOCK) {
ASSERT(imap.br_startblock != DELAYSTARTBLOCK);
- mod = do_mod(imap.br_startblock, mp->m_sb.sb_rextsize);
+ div64_u64_rem(imap.br_startblock, mp->m_sb.sb_rextsize, &mod);
if (mod)
*startoffset_fsb += mp->m_sb.sb_rextsize - mod;
}
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 6cda0f08b045..4a2e5e13c569 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2258,7 +2258,7 @@ xfs_ifree_cluster(
*/
ioffset = inum - xic->first_ino;
if ((xic->alloc & XFS_INOBT_MASK(ioffset)) == 0) {
- ASSERT(do_mod(ioffset, inodes_per_cluster) == 0);
+ ASSERT(ioffset % inodes_per_cluster == 0);
continue;
}
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index b0c98d4faa5b..83474c9cede9 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -30,10 +30,10 @@ xfs_aligned_fsb_count(
if (extsz) {
xfs_extlen_t align;
- align = do_mod(offset_fsb, extsz);
+ div_u64_rem(offset_fsb, extsz, &align);
if (align)
count_fsb += align;
- align = do_mod(count_fsb, extsz);
+ div_u64_rem(count_fsb, extsz, &align);
if (align)
count_fsb += extsz - align;
}
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index 1631cf4546f2..2c800ffbcffd 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -209,25 +209,6 @@ static inline xfs_dev_t linux_to_xfs_dev_t(dev_t dev)
#define xfs_sort(a,n,s,fn) sort(a,n,s,fn,NULL)
#define xfs_stack_trace() dump_stack()
-/* Side effect free 64 bit mod operation */
-static inline __u32 xfs_do_mod(void *a, __u32 b, int n)
-{
- switch (n) {
- case 4:
- return *(__u32 *)a % b;
- case 8:
- {
- __u64 c = *(__u64 *)a;
- return do_div(c, b);
- }
- }
-
- /* NOTREACHED */
- return 0;
-}
-
-#define do_mod(a, b) xfs_do_mod(&(a), (b), sizeof(a))
-
static inline uint64_t roundup_64(uint64_t x, uint32_t y)
{
x += y - 1;
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 7d897c58b0c8..4405ff21f9a9 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -1235,6 +1235,25 @@ xlog_verify_head(
be32_to_cpu((*rhead)->h_size));
}
+/*
+ * We need to make sure we handle log wrapping properly, so we can't use teh
+ * calculated logbno directly. Make sure it wraps to teh correct bno inside teh
+ * log.
+ *
+ * The log is limited to 32 bit sizes, so we use the appropriate modulus
+ * operation here and cast it back to a 64 bit daddr on return.
+ */
+static inline xfs_daddr_t
+xlog_wrap_logbno(
+ struct xlog *log,
+ xfs_daddr_t bno)
+{
+ int mod;
+
+ div_s64_rem(bno, log->l_logBBsize, &mod);
+ return mod;
+}
+
/*
* Check whether the head of the log points to an unmount record. In other
* words, determine whether the log is clean. If so, update the in-core state
@@ -1283,12 +1302,13 @@ xlog_check_unmount_rec(
} else {
hblks = 1;
}
- after_umount_blk = rhead_blk + hblks + BTOBB(be32_to_cpu(rhead->h_len));
- after_umount_blk = do_mod(after_umount_blk, log->l_logBBsize);
+
+ after_umount_blk = xlog_wrap_logbno(log,
+ rhead_blk + hblks + BTOBB(be32_to_cpu(rhead->h_len)));
+
if (*head_blk == after_umount_blk &&
be32_to_cpu(rhead->h_num_logops) == 1) {
- umount_data_blk = rhead_blk + hblks;
- umount_data_blk = do_mod(umount_data_blk, log->l_logBBsize);
+ umount_data_blk = xlog_wrap_logbno(log, rhead_blk + hblks);
error = xlog_bread(log, umount_data_blk, 1, bp, &offset);
if (error)
return error;
@@ -5459,9 +5479,7 @@ xlog_do_recovery_pass(
*/
if (blk_no + bblks <= log->l_logBBsize ||
blk_no >= log->l_logBBsize) {
- /* mod blk_no in case the header wrapped and
- * pushed it beyond the end of the log */
- rblk_no = do_mod(blk_no, log->l_logBBsize);
+ rblk_no = xlog_wrap_logbno(log, blk_no);
error = xlog_bread(log, rblk_no, bblks, dbp,
&offset);
if (error)
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 80bbfe604ce0..776502a5dcb7 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -301,8 +301,12 @@ xfs_rtallocate_extent_block(
/*
* If size should be a multiple of prod, make that so.
*/
- if (prod > 1 && (p = do_mod(bestlen, prod)))
- bestlen -= p;
+ if (prod > 1) {
+ div_u64_rem(bestlen, prod, &p);
+ if (p)
+ bestlen -= p;
+ }
+
/*
* Allocate besti for bestlen & return that.
*/
@@ -1262,8 +1266,11 @@ xfs_rtpick_extent(
resid = seq - (1ULL << log2);
b = (mp->m_sb.sb_rextents * ((resid << 1) + 1ULL)) >>
(log2 + 1);
- if (b >= mp->m_sb.sb_rextents)
- b = do_mod(b, mp->m_sb.sb_rextents);
+ if (b >= mp->m_sb.sb_rextents) {
+ xfs_rtblock_t mod;
+ div64_u64_rem(b, mp->m_sb.sb_rextents, &mod);
+ b = mod;
+ }
if (b + len > mp->m_sb.sb_rextents)
b = mp->m_sb.sb_rextents - len;
}
--
2.17.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/3] xfs: clean up MIN/MAX
2018-06-07 5:27 [PATCH 0/3] xfs: minor cleanups Dave Chinner
2018-06-07 5:27 ` [PATCH 1/3] xfs: move various type verifiers to common file Dave Chinner
2018-06-07 5:27 ` [PATCH 2/3] xfs: replace do_mod with native operations Dave Chinner
@ 2018-06-07 5:27 ` Dave Chinner
2018-06-07 11:42 ` Brian Foster
2018-06-08 6:23 ` Christoph Hellwig
2 siblings, 2 replies; 17+ messages in thread
From: Dave Chinner @ 2018-06-07 5:27 UTC (permalink / raw)
To: linux-xfs
From: Dave Chinner <dchinner@redhat.com>
Get rid of the MIN/MAX macros and just use the native min/max macros
directly in the XFS code.
Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_attr_leaf.c | 2 +-
fs/xfs/libxfs/xfs_bmap.c | 2 +-
fs/xfs/libxfs/xfs_da_btree.c | 2 +-
fs/xfs/libxfs/xfs_dir2_block.c | 8 ++++----
fs/xfs/libxfs/xfs_dir2_leaf.c | 8 ++++----
fs/xfs/libxfs/xfs_sb.c | 2 +-
fs/xfs/libxfs/xfs_trans_resv.c | 16 ++++++++--------
fs/xfs/xfs_buf_item.c | 2 +-
fs/xfs/xfs_iomap.c | 6 +++---
fs/xfs/xfs_itable.c | 2 +-
fs/xfs/xfs_linux.h | 2 --
fs/xfs/xfs_log.c | 4 ++--
fs/xfs/xfs_log_recover.c | 6 +++---
fs/xfs/xfs_mount.h | 2 +-
fs/xfs/xfs_super.c | 2 +-
15 files changed, 32 insertions(+), 34 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index cf1504208811..99e0f5749dba 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -465,7 +465,7 @@ xfs_attr_shortform_bytesfit(xfs_inode_t *dp, int bytes)
* A data fork btree root must have space for at least
* MINDBTPTRS key/ptr pairs if the data fork is small or empty.
*/
- minforkoff = MAX(dsize, XFS_BMDR_SPACE_CALC(MINDBTPTRS));
+ minforkoff = max(dsize, XFS_BMDR_SPACE_CALC(MINDBTPTRS));
minforkoff = roundup(minforkoff, 8) >> 3;
/* attr fork btree root can have at least this many key/ptr pairs */
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 4d8fd37ec7ae..01628f0c9a0c 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3467,7 +3467,7 @@ xfs_bmap_btalloc(
xfs_rmap_skip_owner_update(&args.oinfo);
/* Trim the allocation back to the maximum an AG can fit. */
- args.maxlen = MIN(ap->length, mp->m_ag_max_usable);
+ args.maxlen = min(ap->length, mp->m_ag_max_usable);
args.firstblock = *ap->firstblock;
blen = 0;
if (nullfb) {
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 5d5955d1f775..8a301402bbc4 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -2081,7 +2081,7 @@ xfs_da_grow_inode_int(
*/
mapp = kmem_alloc(sizeof(*mapp) * count, KM_SLEEP);
for (b = *bno, mapi = 0; b < *bno + count; ) {
- nmap = MIN(XFS_BMAP_MAX_NMAP, count);
+ nmap = min(XFS_BMAP_MAX_NMAP, count);
c = (int)(*bno + count - b);
error = xfs_bmapi_write(tp, dp, b, c,
xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA,
diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
index bac4c93e6381..30ed5919da72 100644
--- a/fs/xfs/libxfs/xfs_dir2_block.c
+++ b/fs/xfs/libxfs/xfs_dir2_block.c
@@ -502,8 +502,8 @@ xfs_dir2_block_addname(
if (mid - lowstale)
memmove(&blp[lowstale], &blp[lowstale + 1],
(mid - lowstale) * sizeof(*blp));
- lfloglow = MIN(lowstale, lfloglow);
- lfloghigh = MAX(mid, lfloghigh);
+ lfloglow = min(lowstale, lfloglow);
+ lfloghigh = max(mid, lfloghigh);
}
/*
* Move entries toward the high-numbered stale entry.
@@ -514,8 +514,8 @@ xfs_dir2_block_addname(
if (highstale - mid)
memmove(&blp[mid + 1], &blp[mid],
(highstale - mid) * sizeof(*blp));
- lfloglow = MIN(mid, lfloglow);
- lfloghigh = MAX(highstale, lfloghigh);
+ lfloglow = min(mid, lfloglow);
+ lfloghigh = max(highstale, lfloghigh);
}
be32_add_cpu(&btp->stale, -1);
}
diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index 728c3428abe3..1728a3e6f5cf 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
@@ -590,8 +590,8 @@ xfs_dir3_leaf_find_entry(
(index - lowstale - 1) *
sizeof(xfs_dir2_leaf_entry_t));
}
- *lfloglow = MIN(lowstale, *lfloglow);
- *lfloghigh = MAX(index - 1, *lfloghigh);
+ *lfloglow = min(lowstale, *lfloglow);
+ *lfloghigh = max(index - 1, *lfloghigh);
leafhdr->stale--;
return &ents[index - 1];
}
@@ -610,8 +610,8 @@ xfs_dir3_leaf_find_entry(
memmove(&ents[index + 1], &ents[index],
(highstale - index) * sizeof(xfs_dir2_leaf_entry_t));
}
- *lfloglow = MIN(index, *lfloglow);
- *lfloghigh = MAX(highstale, *lfloghigh);
+ *lfloglow = min(index, *lfloglow);
+ *lfloghigh = max(highstale, *lfloghigh);
leafhdr->stale--;
return &ents[index];
}
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index a63788661a4c..350119eeaecb 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -771,7 +771,7 @@ xfs_sb_mount_common(
mp->m_refc_mnr[1] = mp->m_refc_mxr[1] / 2;
mp->m_bsize = XFS_FSB_TO_BB(mp, 1);
- mp->m_ialloc_inos = (int)MAX((uint16_t)XFS_INODES_PER_CHUNK,
+ mp->m_ialloc_inos = max_t(uint16_t, XFS_INODES_PER_CHUNK,
sbp->sb_inopblock);
mp->m_ialloc_blks = mp->m_ialloc_inos >> sbp->sb_inopblog;
diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
index 50c44e3c0bc5..f99a7aefe418 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.c
+++ b/fs/xfs/libxfs/xfs_trans_resv.c
@@ -236,7 +236,7 @@ xfs_calc_write_reservation(
struct xfs_mount *mp)
{
return XFS_DQUOT_LOGRES(mp) +
- MAX((xfs_calc_inode_res(mp, 1) +
+ max((xfs_calc_inode_res(mp, 1) +
xfs_calc_buf_res(XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK),
XFS_FSB_TO_B(mp, 1)) +
xfs_calc_buf_res(3, mp->m_sb.sb_sectsize) +
@@ -263,7 +263,7 @@ xfs_calc_itruncate_reservation(
struct xfs_mount *mp)
{
return XFS_DQUOT_LOGRES(mp) +
- MAX((xfs_calc_inode_res(mp, 1) +
+ max((xfs_calc_inode_res(mp, 1) +
xfs_calc_buf_res(XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) + 1,
XFS_FSB_TO_B(mp, 1))),
(xfs_calc_buf_res(9, mp->m_sb.sb_sectsize) +
@@ -288,7 +288,7 @@ xfs_calc_rename_reservation(
struct xfs_mount *mp)
{
return XFS_DQUOT_LOGRES(mp) +
- MAX((xfs_calc_inode_res(mp, 4) +
+ max((xfs_calc_inode_res(mp, 4) +
xfs_calc_buf_res(2 * XFS_DIROP_LOG_COUNT(mp),
XFS_FSB_TO_B(mp, 1))),
(xfs_calc_buf_res(7, mp->m_sb.sb_sectsize) +
@@ -328,7 +328,7 @@ xfs_calc_link_reservation(
{
return XFS_DQUOT_LOGRES(mp) +
xfs_calc_iunlink_remove_reservation(mp) +
- MAX((xfs_calc_inode_res(mp, 2) +
+ max((xfs_calc_inode_res(mp, 2) +
xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp),
XFS_FSB_TO_B(mp, 1))),
(xfs_calc_buf_res(3, mp->m_sb.sb_sectsize) +
@@ -366,7 +366,7 @@ xfs_calc_remove_reservation(
{
return XFS_DQUOT_LOGRES(mp) +
xfs_calc_iunlink_add_reservation(mp) +
- MAX((xfs_calc_inode_res(mp, 1) +
+ max((xfs_calc_inode_res(mp, 1) +
xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp),
XFS_FSB_TO_B(mp, 1))),
(xfs_calc_buf_res(4, mp->m_sb.sb_sectsize) +
@@ -424,7 +424,7 @@ STATIC uint
xfs_calc_icreate_reservation(xfs_mount_t *mp)
{
return XFS_DQUOT_LOGRES(mp) +
- MAX(xfs_calc_icreate_resv_alloc(mp),
+ max(xfs_calc_icreate_resv_alloc(mp),
xfs_calc_create_resv_modify(mp));
}
@@ -632,7 +632,7 @@ STATIC uint
xfs_calc_attrinval_reservation(
struct xfs_mount *mp)
{
- return MAX((xfs_calc_inode_res(mp, 1) +
+ return max((xfs_calc_inode_res(mp, 1) +
xfs_calc_buf_res(XFS_BM_MAXLEVELS(mp, XFS_ATTR_FORK),
XFS_FSB_TO_B(mp, 1))),
(xfs_calc_buf_res(9, mp->m_sb.sb_sectsize) +
@@ -696,7 +696,7 @@ xfs_calc_attrrm_reservation(
struct xfs_mount *mp)
{
return XFS_DQUOT_LOGRES(mp) +
- MAX((xfs_calc_inode_res(mp, 1) +
+ max((xfs_calc_inode_res(mp, 1) +
xfs_calc_buf_res(XFS_DA_NODE_MAXDEPTH,
XFS_FSB_TO_B(mp, 1)) +
(uint)XFS_FSB_TO_B(mp,
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 5d18c8089499..1c9d1398980b 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -830,7 +830,7 @@ xfs_buf_item_log_segment(
* of the last bit to be set in this word plus one.
*/
if (bit) {
- end_bit = MIN(bit + bits_to_set, (uint)NBWORD);
+ end_bit = min(bit + bits_to_set, (uint)NBWORD);
mask = ((1U << (end_bit - bit)) - 1) << bit;
*wordp |= mask;
wordp++;
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 99a1a1052885..49f5492eed3b 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -188,7 +188,7 @@ xfs_iomap_write_direct(
goto out_unlock;
} else {
if (nmaps && (imap->br_startblock == HOLESTARTBLOCK))
- last_fsb = MIN(last_fsb, (xfs_fileoff_t)
+ last_fsb = min(last_fsb, (xfs_fileoff_t)
imap->br_blockcount +
imap->br_startoff);
}
@@ -476,8 +476,8 @@ xfs_iomap_prealloc_size(
* The shift throttle value is set to the maximum value as determined by
* the global low free space values and per-quota low free space values.
*/
- alloc_blocks = MIN(alloc_blocks, qblocks);
- shift = MAX(shift, qshift);
+ alloc_blocks = min(alloc_blocks, qblocks);
+ shift = max(shift, qshift);
if (shift)
alloc_blocks >>= shift;
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index fa405f3a00dc..24f4f1c555b5 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -559,7 +559,7 @@ xfs_inumbers(
*lastino != XFS_AGINO_TO_INO(mp, agno, agino))
return error;
- bcount = MIN(left, (int)(PAGE_SIZE / sizeof(*buffer)));
+ bcount = min(left, (int)(PAGE_SIZE / sizeof(*buffer)));
buffer = kmem_zalloc(bcount * sizeof(*buffer), KM_SLEEP);
do {
struct xfs_inobt_rec_incore r;
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index 2c800ffbcffd..edbd5a210df2 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -140,8 +140,6 @@ typedef __u32 xfs_nlink_t;
#define XFS_PROJID_DEFAULT 0
-#define MIN(a,b) (min(a,b))
-#define MAX(a,b) (max(a,b))
#define howmany(x, y) (((x)+((y)-1))/(y))
static inline void delay(long ticks)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index e630778c08b6..5e56f3b93d4b 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1629,8 +1629,8 @@ xlog_grant_push_ail(
* log, and 256 blocks.
*/
free_threshold = BTOBB(need_bytes);
- free_threshold = MAX(free_threshold, (log->l_logBBsize >> 2));
- free_threshold = MAX(free_threshold, 256);
+ free_threshold = max(free_threshold, (log->l_logBBsize >> 2));
+ free_threshold = max(free_threshold, 256);
if (free_blocks >= free_threshold)
return;
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 4405ff21f9a9..8eda116e7812 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -1824,7 +1824,7 @@ xlog_clear_stale_blocks(
* we don't waste all day writing from the head to the tail
* for no reason.
*/
- max_distance = MIN(max_distance, tail_distance);
+ max_distance = min(max_distance, tail_distance);
if ((head_block + max_distance) <= log->l_logBBsize) {
/*
@@ -2892,14 +2892,14 @@ xlog_recover_buffer_pass2(
* buffers in the log can be a different size if the log was generated
* by an older kernel using unclustered inode buffers or a newer kernel
* running with a different inode cluster size. Regardless, if the
- * the inode buffer size isn't MAX(blocksize, mp->m_inode_cluster_size)
+ * the inode buffer size isn't max(blocksize, mp->m_inode_cluster_size)
* for *our* value of mp->m_inode_cluster_size, then we need to keep
* the buffer out of the buffer cache so that the buffer won't
* overlap with future reads of those inodes.
*/
if (XFS_DINODE_MAGIC ==
be16_to_cpu(*((__be16 *)xfs_buf_offset(bp, 0))) &&
- (BBTOB(bp->b_io_length) != MAX(log->l_mp->m_sb.sb_blocksize,
+ (BBTOB(bp->b_io_length) != max(log->l_mp->m_sb.sb_blocksize,
(uint32_t)log->l_mp->m_inode_cluster_size))) {
xfs_buf_stale(bp);
error = xfs_bwrite(bp);
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 7f3d5e012ba3..245349d1e23f 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -271,7 +271,7 @@ xfs_preferred_iosize(xfs_mount_t *mp)
return (mp->m_swidth ?
(mp->m_swidth << mp->m_sb.sb_blocklog) :
((mp->m_flags & XFS_MOUNT_DFLT_IOSIZE) ?
- (1 << (int)MAX(mp->m_readio_log, mp->m_writeio_log)) :
+ (1 << (int)max(mp->m_readio_log, mp->m_writeio_log)) :
PAGE_SIZE));
}
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index d55435e94ea3..7c4813a4a9af 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1136,7 +1136,7 @@ xfs_fs_statfs(
statp->f_bavail = statp->f_bfree;
fakeinos = statp->f_bfree << sbp->sb_inopblog;
- statp->f_files = MIN(icount + fakeinos, (uint64_t)XFS_MAXINUMBER);
+ statp->f_files = min(icount + fakeinos, (uint64_t)XFS_MAXINUMBER);
if (mp->m_maxicount)
statp->f_files = min_t(typeof(statp->f_files),
statp->f_files,
--
2.17.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] xfs: move various type verifiers to common file
2018-06-07 5:27 ` [PATCH 1/3] xfs: move various type verifiers to common file Dave Chinner
@ 2018-06-07 11:41 ` Brian Foster
2018-06-07 15:23 ` Darrick J. Wong
2018-06-08 6:24 ` Christoph Hellwig
2 siblings, 0 replies; 17+ messages in thread
From: Brian Foster @ 2018-06-07 11:41 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Thu, Jun 07, 2018 at 03:27:49PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> New verification functions like xfs_verify_fsbno() and
> xfs_verify_agino() are spread across multiple files and different
> header files. They really don't fit cleanly into the places they've
> been put, and have wider scope than the current header includes.
>
> Move the type verifiers to a new file in libxfs (xfs-types.c) and
> the prototypes to xfs_types.h where they will be visible to all the
> code that uses the types.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/Makefile | 1 +
> fs/xfs/libxfs/xfs_alloc.c | 49 ----------
> fs/xfs/libxfs/xfs_alloc.h | 4 -
> fs/xfs/libxfs/xfs_ialloc.c | 90 ------------------
> fs/xfs/libxfs/xfs_ialloc.h | 7 --
> fs/xfs/libxfs/xfs_rtbitmap.c | 12 ---
> fs/xfs/libxfs/xfs_types.c | 173 +++++++++++++++++++++++++++++++++++
> fs/xfs/libxfs/xfs_types.h | 19 ++++
> fs/xfs/scrub/agheader.c | 2 +-
> 9 files changed, 194 insertions(+), 163 deletions(-)
> create mode 100644 fs/xfs/libxfs/xfs_types.c
>
> diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
> index 19a1f8064d8a..5333f7cbc95a 100644
> --- a/fs/xfs/Makefile
> +++ b/fs/xfs/Makefile
> @@ -50,6 +50,7 @@ xfs-y += $(addprefix libxfs/, \
> xfs_sb.o \
> xfs_symlink_remote.o \
> xfs_trans_resv.o \
> + xfs_types.o \
> )
> # xfs_rtbitmap is shared with libxfs
> xfs-$(CONFIG_XFS_RT) += $(addprefix libxfs/, \
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 1db50cfc0212..eef466260d43 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -3123,55 +3123,6 @@ xfs_alloc_query_all(
> return xfs_btree_query_all(cur, xfs_alloc_query_range_helper, &query);
> }
>
> -/* Find the size of the AG, in blocks. */
> -xfs_agblock_t
> -xfs_ag_block_count(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno)
> -{
> - ASSERT(agno < mp->m_sb.sb_agcount);
> -
> - if (agno < mp->m_sb.sb_agcount - 1)
> - return mp->m_sb.sb_agblocks;
> - return mp->m_sb.sb_dblocks - (agno * mp->m_sb.sb_agblocks);
> -}
> -
> -/*
> - * Verify that an AG block number pointer neither points outside the AG
> - * nor points at static metadata.
> - */
> -bool
> -xfs_verify_agbno(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno,
> - xfs_agblock_t agbno)
> -{
> - xfs_agblock_t eoag;
> -
> - eoag = xfs_ag_block_count(mp, agno);
> - if (agbno >= eoag)
> - return false;
> - if (agbno <= XFS_AGFL_BLOCK(mp))
> - return false;
> - return true;
> -}
> -
> -/*
> - * Verify that an FS block number pointer neither points outside the
> - * filesystem nor points at static AG metadata.
> - */
> -bool
> -xfs_verify_fsbno(
> - struct xfs_mount *mp,
> - xfs_fsblock_t fsbno)
> -{
> - xfs_agnumber_t agno = XFS_FSB_TO_AGNO(mp, fsbno);
> -
> - if (agno >= mp->m_sb.sb_agcount)
> - return false;
> - return xfs_verify_agbno(mp, agno, XFS_FSB_TO_AGBNO(mp, fsbno));
> -}
> -
> /* Is there a record covering a given extent? */
> int
> xfs_alloc_has_record(
> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
> index 1651d924aaf1..e716c993ac4c 100644
> --- a/fs/xfs/libxfs/xfs_alloc.h
> +++ b/fs/xfs/libxfs/xfs_alloc.h
> @@ -242,10 +242,6 @@ int xfs_alloc_query_range(struct xfs_btree_cur *cur,
> xfs_alloc_query_range_fn fn, void *priv);
> int xfs_alloc_query_all(struct xfs_btree_cur *cur, xfs_alloc_query_range_fn fn,
> void *priv);
> -xfs_agblock_t xfs_ag_block_count(struct xfs_mount *mp, xfs_agnumber_t agno);
> -bool xfs_verify_agbno(struct xfs_mount *mp, xfs_agnumber_t agno,
> - xfs_agblock_t agbno);
> -bool xfs_verify_fsbno(struct xfs_mount *mp, xfs_fsblock_t fsbno);
>
> int xfs_alloc_has_record(struct xfs_btree_cur *cur, xfs_agblock_t bno,
> xfs_extlen_t len, bool *exist);
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 3f551eb29157..8ec39dad62d7 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -2674,96 +2674,6 @@ xfs_ialloc_pagi_init(
> return 0;
> }
>
> -/* Calculate the first and last possible inode number in an AG. */
> -void
> -xfs_ialloc_agino_range(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno,
> - xfs_agino_t *first,
> - xfs_agino_t *last)
> -{
> - xfs_agblock_t bno;
> - xfs_agblock_t eoag;
> -
> - eoag = xfs_ag_block_count(mp, agno);
> -
> - /*
> - * Calculate the first inode, which will be in the first
> - * cluster-aligned block after the AGFL.
> - */
> - bno = round_up(XFS_AGFL_BLOCK(mp) + 1,
> - xfs_ialloc_cluster_alignment(mp));
> - *first = XFS_OFFBNO_TO_AGINO(mp, bno, 0);
> -
> - /*
> - * Calculate the last inode, which will be at the end of the
> - * last (aligned) cluster that can be allocated in the AG.
> - */
> - bno = round_down(eoag, xfs_ialloc_cluster_alignment(mp));
> - *last = XFS_OFFBNO_TO_AGINO(mp, bno, 0) - 1;
> -}
> -
> -/*
> - * Verify that an AG inode number pointer neither points outside the AG
> - * nor points at static metadata.
> - */
> -bool
> -xfs_verify_agino(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno,
> - xfs_agino_t agino)
> -{
> - xfs_agino_t first;
> - xfs_agino_t last;
> -
> - xfs_ialloc_agino_range(mp, agno, &first, &last);
> - return agino >= first && agino <= last;
> -}
> -
> -/*
> - * Verify that an FS inode number pointer neither points outside the
> - * filesystem nor points at static AG metadata.
> - */
> -bool
> -xfs_verify_ino(
> - struct xfs_mount *mp,
> - xfs_ino_t ino)
> -{
> - xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, ino);
> - xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ino);
> -
> - if (agno >= mp->m_sb.sb_agcount)
> - return false;
> - if (XFS_AGINO_TO_INO(mp, agno, agino) != ino)
> - return false;
> - return xfs_verify_agino(mp, agno, agino);
> -}
> -
> -/* Is this an internal inode number? */
> -bool
> -xfs_internal_inum(
> - struct xfs_mount *mp,
> - xfs_ino_t ino)
> -{
> - return ino == mp->m_sb.sb_rbmino || ino == mp->m_sb.sb_rsumino ||
> - (xfs_sb_version_hasquota(&mp->m_sb) &&
> - xfs_is_quota_inode(&mp->m_sb, ino));
> -}
> -
> -/*
> - * Verify that a directory entry's inode number doesn't point at an internal
> - * inode, empty space, or static AG metadata.
> - */
> -bool
> -xfs_verify_dir_ino(
> - struct xfs_mount *mp,
> - xfs_ino_t ino)
> -{
> - if (xfs_internal_inum(mp, ino))
> - return false;
> - return xfs_verify_ino(mp, ino);
> -}
> -
> /* Is there an inode record covering a given range of inode numbers? */
> int
> xfs_ialloc_has_inode_record(
> diff --git a/fs/xfs/libxfs/xfs_ialloc.h b/fs/xfs/libxfs/xfs_ialloc.h
> index 144f3eac9b93..90b09c5f163b 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.h
> +++ b/fs/xfs/libxfs/xfs_ialloc.h
> @@ -169,12 +169,5 @@ int xfs_inobt_insert_rec(struct xfs_btree_cur *cur, uint16_t holemask,
> int *stat);
>
> int xfs_ialloc_cluster_alignment(struct xfs_mount *mp);
> -void xfs_ialloc_agino_range(struct xfs_mount *mp, xfs_agnumber_t agno,
> - xfs_agino_t *first, xfs_agino_t *last);
> -bool xfs_verify_agino(struct xfs_mount *mp, xfs_agnumber_t agno,
> - xfs_agino_t agino);
> -bool xfs_verify_ino(struct xfs_mount *mp, xfs_ino_t ino);
> -bool xfs_internal_inum(struct xfs_mount *mp, xfs_ino_t ino);
> -bool xfs_verify_dir_ino(struct xfs_mount *mp, xfs_ino_t ino);
>
> #endif /* __XFS_IALLOC_H__ */
> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
> index ffc72075a44e..65fc4ed2e9a1 100644
> --- a/fs/xfs/libxfs/xfs_rtbitmap.c
> +++ b/fs/xfs/libxfs/xfs_rtbitmap.c
> @@ -1080,18 +1080,6 @@ xfs_rtalloc_query_all(
> return xfs_rtalloc_query_range(tp, &keys[0], &keys[1], fn, priv);
> }
>
> -/*
> - * Verify that an realtime block number pointer doesn't point off the
> - * end of the realtime device.
> - */
> -bool
> -xfs_verify_rtbno(
> - struct xfs_mount *mp,
> - xfs_rtblock_t rtbno)
> -{
> - return rtbno < mp->m_sb.sb_rblocks;
> -}
> -
> /* Is the given extent all free? */
> int
> xfs_rtalloc_extent_is_free(
> diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
> new file mode 100644
> index 000000000000..2e2a243cef2e
> --- /dev/null
> +++ b/fs/xfs/libxfs/xfs_types.c
> @@ -0,0 +1,173 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2000-2002,2005 Silicon Graphics, Inc.
> + * Copyright (C) 2017 Oracle.
> + * All Rights Reserved.
> + */
> +#include "xfs.h"
> +#include "xfs_fs.h"
> +#include "xfs_format.h"
> +#include "xfs_log_format.h"
> +#include "xfs_shared.h"
> +#include "xfs_trans_resv.h"
> +#include "xfs_bit.h"
> +#include "xfs_sb.h"
> +#include "xfs_mount.h"
> +#include "xfs_defer.h"
> +#include "xfs_inode.h"
> +#include "xfs_btree.h"
> +#include "xfs_rmap.h"
> +#include "xfs_alloc_btree.h"
> +#include "xfs_alloc.h"
> +#include "xfs_ialloc.h"
> +
> +/* Find the size of the AG, in blocks. */
> +xfs_agblock_t
> +xfs_ag_block_count(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno)
> +{
> + ASSERT(agno < mp->m_sb.sb_agcount);
> +
> + if (agno < mp->m_sb.sb_agcount - 1)
> + return mp->m_sb.sb_agblocks;
> + return mp->m_sb.sb_dblocks - (agno * mp->m_sb.sb_agblocks);
> +}
> +
> +/*
> + * Verify that an AG block number pointer neither points outside the AG
> + * nor points at static metadata.
> + */
> +bool
> +xfs_verify_agbno(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno,
> + xfs_agblock_t agbno)
> +{
> + xfs_agblock_t eoag;
> +
> + eoag = xfs_ag_block_count(mp, agno);
> + if (agbno >= eoag)
> + return false;
> + if (agbno <= XFS_AGFL_BLOCK(mp))
> + return false;
> + return true;
> +}
> +
> +/*
> + * Verify that an FS block number pointer neither points outside the
> + * filesystem nor points at static AG metadata.
> + */
> +bool
> +xfs_verify_fsbno(
> + struct xfs_mount *mp,
> + xfs_fsblock_t fsbno)
> +{
> + xfs_agnumber_t agno = XFS_FSB_TO_AGNO(mp, fsbno);
> +
> + if (agno >= mp->m_sb.sb_agcount)
> + return false;
> + return xfs_verify_agbno(mp, agno, XFS_FSB_TO_AGBNO(mp, fsbno));
> +}
> +
> +/* Calculate the first and last possible inode number in an AG. */
> +void
> +xfs_agino_range(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno,
> + xfs_agino_t *first,
> + xfs_agino_t *last)
> +{
> + xfs_agblock_t bno;
> + xfs_agblock_t eoag;
> +
> + eoag = xfs_ag_block_count(mp, agno);
> +
> + /*
> + * Calculate the first inode, which will be in the first
> + * cluster-aligned block after the AGFL.
> + */
> + bno = round_up(XFS_AGFL_BLOCK(mp) + 1,
> + xfs_ialloc_cluster_alignment(mp));
> + *first = XFS_OFFBNO_TO_AGINO(mp, bno, 0);
> +
> + /*
> + * Calculate the last inode, which will be at the end of the
> + * last (aligned) cluster that can be allocated in the AG.
> + */
> + bno = round_down(eoag, xfs_ialloc_cluster_alignment(mp));
> + *last = XFS_OFFBNO_TO_AGINO(mp, bno, 0) - 1;
> +}
> +
> +/*
> + * Verify that an AG inode number pointer neither points outside the AG
> + * nor points at static metadata.
> + */
> +bool
> +xfs_verify_agino(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno,
> + xfs_agino_t agino)
> +{
> + xfs_agino_t first;
> + xfs_agino_t last;
> +
> + xfs_agino_range(mp, agno, &first, &last);
> + return agino >= first && agino <= last;
> +}
> +
> +/*
> + * Verify that an FS inode number pointer neither points outside the
> + * filesystem nor points at static AG metadata.
> + */
> +bool
> +xfs_verify_ino(
> + struct xfs_mount *mp,
> + xfs_ino_t ino)
> +{
> + xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, ino);
> + xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ino);
> +
> + if (agno >= mp->m_sb.sb_agcount)
> + return false;
> + if (XFS_AGINO_TO_INO(mp, agno, agino) != ino)
> + return false;
> + return xfs_verify_agino(mp, agno, agino);
> +}
> +
> +/* Is this an internal inode number? */
> +bool
> +xfs_internal_inum(
> + struct xfs_mount *mp,
> + xfs_ino_t ino)
> +{
> + return ino == mp->m_sb.sb_rbmino || ino == mp->m_sb.sb_rsumino ||
> + (xfs_sb_version_hasquota(&mp->m_sb) &&
> + xfs_is_quota_inode(&mp->m_sb, ino));
> +}
> +
> +/*
> + * Verify that a directory entry's inode number doesn't point at an internal
> + * inode, empty space, or static AG metadata.
> + */
> +bool
> +xfs_verify_dir_ino(
> + struct xfs_mount *mp,
> + xfs_ino_t ino)
> +{
> + if (xfs_internal_inum(mp, ino))
> + return false;
> + return xfs_verify_ino(mp, ino);
> +}
> +
> +/*
> + * Verify that an realtime block number pointer doesn't point off the
> + * end of the realtime device.
> + */
> +bool
> +xfs_verify_rtbno(
> + struct xfs_mount *mp,
> + xfs_rtblock_t rtbno)
> +{
> + return rtbno < mp->m_sb.sb_rblocks;
> +}
> diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
> index b72ae343140e..4055d62f690c 100644
> --- a/fs/xfs/libxfs/xfs_types.h
> +++ b/fs/xfs/libxfs/xfs_types.h
> @@ -147,4 +147,23 @@ typedef struct xfs_bmbt_irec
> xfs_exntst_t br_state; /* extent state */
> } xfs_bmbt_irec_t;
>
> +/*
> + * Type verifier functions
> + */
> +struct xfs_mount;
> +
> +xfs_agblock_t xfs_ag_block_count(struct xfs_mount *mp, xfs_agnumber_t agno);
> +bool xfs_verify_agbno(struct xfs_mount *mp, xfs_agnumber_t agno,
> + xfs_agblock_t agbno);
> +bool xfs_verify_fsbno(struct xfs_mount *mp, xfs_fsblock_t fsbno);
> +
> +void xfs_agino_range(struct xfs_mount *mp, xfs_agnumber_t agno,
> + xfs_agino_t *first, xfs_agino_t *last);
> +bool xfs_verify_agino(struct xfs_mount *mp, xfs_agnumber_t agno,
> + xfs_agino_t agino);
> +bool xfs_verify_ino(struct xfs_mount *mp, xfs_ino_t ino);
> +bool xfs_internal_inum(struct xfs_mount *mp, xfs_ino_t ino);
> +bool xfs_verify_dir_ino(struct xfs_mount *mp, xfs_ino_t ino);
> +bool xfs_verify_rtbno(struct xfs_mount *mp, xfs_rtblock_t rtbno);
> +
> #endif /* __XFS_TYPES_H__ */
> diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
> index fb9637ff4bde..9bb0745f1ad2 100644
> --- a/fs/xfs/scrub/agheader.c
> +++ b/fs/xfs/scrub/agheader.c
> @@ -867,7 +867,7 @@ xfs_scrub_agi(
> }
>
> /* Check inode counters */
> - xfs_ialloc_agino_range(mp, agno, &first_agino, &last_agino);
> + xfs_agino_range(mp, agno, &first_agino, &last_agino);
> icount = be32_to_cpu(agi->agi_count);
> if (icount > last_agino - first_agino + 1 ||
> icount < be32_to_cpu(agi->agi_freecount))
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] xfs: replace do_mod with native operations
2018-06-07 5:27 ` [PATCH 2/3] xfs: replace do_mod with native operations Dave Chinner
@ 2018-06-07 11:42 ` Brian Foster
2018-06-07 16:01 ` Darrick J. Wong
2018-06-07 22:23 ` Dave Chinner
2018-06-07 15:54 ` Darrick J. Wong
2018-06-08 0:43 ` [PATCH 2/3 V2] " Dave Chinner
2 siblings, 2 replies; 17+ messages in thread
From: Brian Foster @ 2018-06-07 11:42 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Thu, Jun 07, 2018 at 03:27:50PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> do_mod() is a hold-over from when we have different sizes for file
> offsets and and other internal values for 40 bit XFS filesystems.
> Hence depending on build flags variables passed to do_mod() could
> change size. We no longer support those small format filesystems and
> hence everything is of fixed size theses days, even on 32 bit
> platforms.
>
> As such, we can convert all the do_mod() callers to platform
> optimised modulus operations as defined by linux/math64.h.
> Individual conversions depend on the types of variables being used.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 37 +++++++++++++++++++++++--------------
> fs/xfs/xfs_bmap_util.c | 14 +++++++++-----
> fs/xfs/xfs_inode.c | 2 +-
> fs/xfs/xfs_iomap.h | 4 ++--
> fs/xfs/xfs_linux.h | 19 -------------------
> fs/xfs/xfs_log_recover.c | 32 +++++++++++++++++++++++++-------
> fs/xfs/xfs_rtalloc.c | 15 +++++++++++----
> 7 files changed, 71 insertions(+), 52 deletions(-)
>
...
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 7d897c58b0c8..4405ff21f9a9 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -1235,6 +1235,25 @@ xlog_verify_head(
> be32_to_cpu((*rhead)->h_size));
> }
>
> +/*
> + * We need to make sure we handle log wrapping properly, so we can't use teh
> + * calculated logbno directly. Make sure it wraps to teh correct bno inside teh
> + * log.
> + *
s/teh/the/
> + * The log is limited to 32 bit sizes, so we use the appropriate modulus
> + * operation here and cast it back to a 64 bit daddr on return.
> + */
> +static inline xfs_daddr_t
> +xlog_wrap_logbno(
> + struct xlog *log,
> + xfs_daddr_t bno)
> +{
> + int mod;
> +
> + div_s64_rem(bno, log->l_logBBsize, &mod);
> + return mod;
> +}
> +
> /*
> * Check whether the head of the log points to an unmount record. In other
> * words, determine whether the log is clean. If so, update the in-core state
...
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 80bbfe604ce0..776502a5dcb7 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
...
> @@ -1262,8 +1266,11 @@ xfs_rtpick_extent(
> resid = seq - (1ULL << log2);
> b = (mp->m_sb.sb_rextents * ((resid << 1) + 1ULL)) >>
> (log2 + 1);
> - if (b >= mp->m_sb.sb_rextents)
> - b = do_mod(b, mp->m_sb.sb_rextents);
> + if (b >= mp->m_sb.sb_rextents) {
> + xfs_rtblock_t mod;
> + div64_u64_rem(b, mp->m_sb.sb_rextents, &mod);
> + b = mod;
> + }
Shouldn't we be able to do 'div64_u64_rem(b, mp->m_sb.sb_rextents, &b)'
here? Otherwise looks fine:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> if (b + len > mp->m_sb.sb_rextents)
> b = mp->m_sb.sb_rextents - len;
> }
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] xfs: clean up MIN/MAX
2018-06-07 5:27 ` [PATCH 3/3] xfs: clean up MIN/MAX Dave Chinner
@ 2018-06-07 11:42 ` Brian Foster
2018-06-08 6:23 ` Christoph Hellwig
1 sibling, 0 replies; 17+ messages in thread
From: Brian Foster @ 2018-06-07 11:42 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Thu, Jun 07, 2018 at 03:27:51PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Get rid of the MIN/MAX macros and just use the native min/max macros
> directly in the XFS code.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/libxfs/xfs_attr_leaf.c | 2 +-
> fs/xfs/libxfs/xfs_bmap.c | 2 +-
> fs/xfs/libxfs/xfs_da_btree.c | 2 +-
> fs/xfs/libxfs/xfs_dir2_block.c | 8 ++++----
> fs/xfs/libxfs/xfs_dir2_leaf.c | 8 ++++----
> fs/xfs/libxfs/xfs_sb.c | 2 +-
> fs/xfs/libxfs/xfs_trans_resv.c | 16 ++++++++--------
> fs/xfs/xfs_buf_item.c | 2 +-
> fs/xfs/xfs_iomap.c | 6 +++---
> fs/xfs/xfs_itable.c | 2 +-
> fs/xfs/xfs_linux.h | 2 --
> fs/xfs/xfs_log.c | 4 ++--
> fs/xfs/xfs_log_recover.c | 6 +++---
> fs/xfs/xfs_mount.h | 2 +-
> fs/xfs/xfs_super.c | 2 +-
> 15 files changed, 32 insertions(+), 34 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index cf1504208811..99e0f5749dba 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -465,7 +465,7 @@ xfs_attr_shortform_bytesfit(xfs_inode_t *dp, int bytes)
> * A data fork btree root must have space for at least
> * MINDBTPTRS key/ptr pairs if the data fork is small or empty.
> */
> - minforkoff = MAX(dsize, XFS_BMDR_SPACE_CALC(MINDBTPTRS));
> + minforkoff = max(dsize, XFS_BMDR_SPACE_CALC(MINDBTPTRS));
> minforkoff = roundup(minforkoff, 8) >> 3;
>
> /* attr fork btree root can have at least this many key/ptr pairs */
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 4d8fd37ec7ae..01628f0c9a0c 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3467,7 +3467,7 @@ xfs_bmap_btalloc(
> xfs_rmap_skip_owner_update(&args.oinfo);
>
> /* Trim the allocation back to the maximum an AG can fit. */
> - args.maxlen = MIN(ap->length, mp->m_ag_max_usable);
> + args.maxlen = min(ap->length, mp->m_ag_max_usable);
> args.firstblock = *ap->firstblock;
> blen = 0;
> if (nullfb) {
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 5d5955d1f775..8a301402bbc4 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -2081,7 +2081,7 @@ xfs_da_grow_inode_int(
> */
> mapp = kmem_alloc(sizeof(*mapp) * count, KM_SLEEP);
> for (b = *bno, mapi = 0; b < *bno + count; ) {
> - nmap = MIN(XFS_BMAP_MAX_NMAP, count);
> + nmap = min(XFS_BMAP_MAX_NMAP, count);
> c = (int)(*bno + count - b);
> error = xfs_bmapi_write(tp, dp, b, c,
> xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA,
> diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
> index bac4c93e6381..30ed5919da72 100644
> --- a/fs/xfs/libxfs/xfs_dir2_block.c
> +++ b/fs/xfs/libxfs/xfs_dir2_block.c
> @@ -502,8 +502,8 @@ xfs_dir2_block_addname(
> if (mid - lowstale)
> memmove(&blp[lowstale], &blp[lowstale + 1],
> (mid - lowstale) * sizeof(*blp));
> - lfloglow = MIN(lowstale, lfloglow);
> - lfloghigh = MAX(mid, lfloghigh);
> + lfloglow = min(lowstale, lfloglow);
> + lfloghigh = max(mid, lfloghigh);
> }
> /*
> * Move entries toward the high-numbered stale entry.
> @@ -514,8 +514,8 @@ xfs_dir2_block_addname(
> if (highstale - mid)
> memmove(&blp[mid + 1], &blp[mid],
> (highstale - mid) * sizeof(*blp));
> - lfloglow = MIN(mid, lfloglow);
> - lfloghigh = MAX(highstale, lfloghigh);
> + lfloglow = min(mid, lfloglow);
> + lfloghigh = max(highstale, lfloghigh);
> }
> be32_add_cpu(&btp->stale, -1);
> }
> diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> index 728c3428abe3..1728a3e6f5cf 100644
> --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> @@ -590,8 +590,8 @@ xfs_dir3_leaf_find_entry(
> (index - lowstale - 1) *
> sizeof(xfs_dir2_leaf_entry_t));
> }
> - *lfloglow = MIN(lowstale, *lfloglow);
> - *lfloghigh = MAX(index - 1, *lfloghigh);
> + *lfloglow = min(lowstale, *lfloglow);
> + *lfloghigh = max(index - 1, *lfloghigh);
> leafhdr->stale--;
> return &ents[index - 1];
> }
> @@ -610,8 +610,8 @@ xfs_dir3_leaf_find_entry(
> memmove(&ents[index + 1], &ents[index],
> (highstale - index) * sizeof(xfs_dir2_leaf_entry_t));
> }
> - *lfloglow = MIN(index, *lfloglow);
> - *lfloghigh = MAX(highstale, *lfloghigh);
> + *lfloglow = min(index, *lfloglow);
> + *lfloghigh = max(highstale, *lfloghigh);
> leafhdr->stale--;
> return &ents[index];
> }
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index a63788661a4c..350119eeaecb 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -771,7 +771,7 @@ xfs_sb_mount_common(
> mp->m_refc_mnr[1] = mp->m_refc_mxr[1] / 2;
>
> mp->m_bsize = XFS_FSB_TO_BB(mp, 1);
> - mp->m_ialloc_inos = (int)MAX((uint16_t)XFS_INODES_PER_CHUNK,
> + mp->m_ialloc_inos = max_t(uint16_t, XFS_INODES_PER_CHUNK,
> sbp->sb_inopblock);
> mp->m_ialloc_blks = mp->m_ialloc_inos >> sbp->sb_inopblog;
>
> diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
> index 50c44e3c0bc5..f99a7aefe418 100644
> --- a/fs/xfs/libxfs/xfs_trans_resv.c
> +++ b/fs/xfs/libxfs/xfs_trans_resv.c
> @@ -236,7 +236,7 @@ xfs_calc_write_reservation(
> struct xfs_mount *mp)
> {
> return XFS_DQUOT_LOGRES(mp) +
> - MAX((xfs_calc_inode_res(mp, 1) +
> + max((xfs_calc_inode_res(mp, 1) +
> xfs_calc_buf_res(XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK),
> XFS_FSB_TO_B(mp, 1)) +
> xfs_calc_buf_res(3, mp->m_sb.sb_sectsize) +
> @@ -263,7 +263,7 @@ xfs_calc_itruncate_reservation(
> struct xfs_mount *mp)
> {
> return XFS_DQUOT_LOGRES(mp) +
> - MAX((xfs_calc_inode_res(mp, 1) +
> + max((xfs_calc_inode_res(mp, 1) +
> xfs_calc_buf_res(XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) + 1,
> XFS_FSB_TO_B(mp, 1))),
> (xfs_calc_buf_res(9, mp->m_sb.sb_sectsize) +
> @@ -288,7 +288,7 @@ xfs_calc_rename_reservation(
> struct xfs_mount *mp)
> {
> return XFS_DQUOT_LOGRES(mp) +
> - MAX((xfs_calc_inode_res(mp, 4) +
> + max((xfs_calc_inode_res(mp, 4) +
> xfs_calc_buf_res(2 * XFS_DIROP_LOG_COUNT(mp),
> XFS_FSB_TO_B(mp, 1))),
> (xfs_calc_buf_res(7, mp->m_sb.sb_sectsize) +
> @@ -328,7 +328,7 @@ xfs_calc_link_reservation(
> {
> return XFS_DQUOT_LOGRES(mp) +
> xfs_calc_iunlink_remove_reservation(mp) +
> - MAX((xfs_calc_inode_res(mp, 2) +
> + max((xfs_calc_inode_res(mp, 2) +
> xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp),
> XFS_FSB_TO_B(mp, 1))),
> (xfs_calc_buf_res(3, mp->m_sb.sb_sectsize) +
> @@ -366,7 +366,7 @@ xfs_calc_remove_reservation(
> {
> return XFS_DQUOT_LOGRES(mp) +
> xfs_calc_iunlink_add_reservation(mp) +
> - MAX((xfs_calc_inode_res(mp, 1) +
> + max((xfs_calc_inode_res(mp, 1) +
> xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp),
> XFS_FSB_TO_B(mp, 1))),
> (xfs_calc_buf_res(4, mp->m_sb.sb_sectsize) +
> @@ -424,7 +424,7 @@ STATIC uint
> xfs_calc_icreate_reservation(xfs_mount_t *mp)
> {
> return XFS_DQUOT_LOGRES(mp) +
> - MAX(xfs_calc_icreate_resv_alloc(mp),
> + max(xfs_calc_icreate_resv_alloc(mp),
> xfs_calc_create_resv_modify(mp));
> }
>
> @@ -632,7 +632,7 @@ STATIC uint
> xfs_calc_attrinval_reservation(
> struct xfs_mount *mp)
> {
> - return MAX((xfs_calc_inode_res(mp, 1) +
> + return max((xfs_calc_inode_res(mp, 1) +
> xfs_calc_buf_res(XFS_BM_MAXLEVELS(mp, XFS_ATTR_FORK),
> XFS_FSB_TO_B(mp, 1))),
> (xfs_calc_buf_res(9, mp->m_sb.sb_sectsize) +
> @@ -696,7 +696,7 @@ xfs_calc_attrrm_reservation(
> struct xfs_mount *mp)
> {
> return XFS_DQUOT_LOGRES(mp) +
> - MAX((xfs_calc_inode_res(mp, 1) +
> + max((xfs_calc_inode_res(mp, 1) +
> xfs_calc_buf_res(XFS_DA_NODE_MAXDEPTH,
> XFS_FSB_TO_B(mp, 1)) +
> (uint)XFS_FSB_TO_B(mp,
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index 5d18c8089499..1c9d1398980b 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -830,7 +830,7 @@ xfs_buf_item_log_segment(
> * of the last bit to be set in this word plus one.
> */
> if (bit) {
> - end_bit = MIN(bit + bits_to_set, (uint)NBWORD);
> + end_bit = min(bit + bits_to_set, (uint)NBWORD);
> mask = ((1U << (end_bit - bit)) - 1) << bit;
> *wordp |= mask;
> wordp++;
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 99a1a1052885..49f5492eed3b 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -188,7 +188,7 @@ xfs_iomap_write_direct(
> goto out_unlock;
> } else {
> if (nmaps && (imap->br_startblock == HOLESTARTBLOCK))
> - last_fsb = MIN(last_fsb, (xfs_fileoff_t)
> + last_fsb = min(last_fsb, (xfs_fileoff_t)
> imap->br_blockcount +
> imap->br_startoff);
> }
> @@ -476,8 +476,8 @@ xfs_iomap_prealloc_size(
> * The shift throttle value is set to the maximum value as determined by
> * the global low free space values and per-quota low free space values.
> */
> - alloc_blocks = MIN(alloc_blocks, qblocks);
> - shift = MAX(shift, qshift);
> + alloc_blocks = min(alloc_blocks, qblocks);
> + shift = max(shift, qshift);
>
> if (shift)
> alloc_blocks >>= shift;
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index fa405f3a00dc..24f4f1c555b5 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -559,7 +559,7 @@ xfs_inumbers(
> *lastino != XFS_AGINO_TO_INO(mp, agno, agino))
> return error;
>
> - bcount = MIN(left, (int)(PAGE_SIZE / sizeof(*buffer)));
> + bcount = min(left, (int)(PAGE_SIZE / sizeof(*buffer)));
> buffer = kmem_zalloc(bcount * sizeof(*buffer), KM_SLEEP);
> do {
> struct xfs_inobt_rec_incore r;
> diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
> index 2c800ffbcffd..edbd5a210df2 100644
> --- a/fs/xfs/xfs_linux.h
> +++ b/fs/xfs/xfs_linux.h
> @@ -140,8 +140,6 @@ typedef __u32 xfs_nlink_t;
>
> #define XFS_PROJID_DEFAULT 0
>
> -#define MIN(a,b) (min(a,b))
> -#define MAX(a,b) (max(a,b))
> #define howmany(x, y) (((x)+((y)-1))/(y))
>
> static inline void delay(long ticks)
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index e630778c08b6..5e56f3b93d4b 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1629,8 +1629,8 @@ xlog_grant_push_ail(
> * log, and 256 blocks.
> */
> free_threshold = BTOBB(need_bytes);
> - free_threshold = MAX(free_threshold, (log->l_logBBsize >> 2));
> - free_threshold = MAX(free_threshold, 256);
> + free_threshold = max(free_threshold, (log->l_logBBsize >> 2));
> + free_threshold = max(free_threshold, 256);
> if (free_blocks >= free_threshold)
> return;
>
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 4405ff21f9a9..8eda116e7812 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -1824,7 +1824,7 @@ xlog_clear_stale_blocks(
> * we don't waste all day writing from the head to the tail
> * for no reason.
> */
> - max_distance = MIN(max_distance, tail_distance);
> + max_distance = min(max_distance, tail_distance);
>
> if ((head_block + max_distance) <= log->l_logBBsize) {
> /*
> @@ -2892,14 +2892,14 @@ xlog_recover_buffer_pass2(
> * buffers in the log can be a different size if the log was generated
> * by an older kernel using unclustered inode buffers or a newer kernel
> * running with a different inode cluster size. Regardless, if the
> - * the inode buffer size isn't MAX(blocksize, mp->m_inode_cluster_size)
> + * the inode buffer size isn't max(blocksize, mp->m_inode_cluster_size)
> * for *our* value of mp->m_inode_cluster_size, then we need to keep
> * the buffer out of the buffer cache so that the buffer won't
> * overlap with future reads of those inodes.
> */
> if (XFS_DINODE_MAGIC ==
> be16_to_cpu(*((__be16 *)xfs_buf_offset(bp, 0))) &&
> - (BBTOB(bp->b_io_length) != MAX(log->l_mp->m_sb.sb_blocksize,
> + (BBTOB(bp->b_io_length) != max(log->l_mp->m_sb.sb_blocksize,
> (uint32_t)log->l_mp->m_inode_cluster_size))) {
> xfs_buf_stale(bp);
> error = xfs_bwrite(bp);
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 7f3d5e012ba3..245349d1e23f 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -271,7 +271,7 @@ xfs_preferred_iosize(xfs_mount_t *mp)
> return (mp->m_swidth ?
> (mp->m_swidth << mp->m_sb.sb_blocklog) :
> ((mp->m_flags & XFS_MOUNT_DFLT_IOSIZE) ?
> - (1 << (int)MAX(mp->m_readio_log, mp->m_writeio_log)) :
> + (1 << (int)max(mp->m_readio_log, mp->m_writeio_log)) :
> PAGE_SIZE));
> }
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index d55435e94ea3..7c4813a4a9af 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1136,7 +1136,7 @@ xfs_fs_statfs(
> statp->f_bavail = statp->f_bfree;
>
> fakeinos = statp->f_bfree << sbp->sb_inopblog;
> - statp->f_files = MIN(icount + fakeinos, (uint64_t)XFS_MAXINUMBER);
> + statp->f_files = min(icount + fakeinos, (uint64_t)XFS_MAXINUMBER);
> if (mp->m_maxicount)
> statp->f_files = min_t(typeof(statp->f_files),
> statp->f_files,
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] xfs: move various type verifiers to common file
2018-06-07 5:27 ` [PATCH 1/3] xfs: move various type verifiers to common file Dave Chinner
2018-06-07 11:41 ` Brian Foster
@ 2018-06-07 15:23 ` Darrick J. Wong
2018-06-08 6:24 ` Christoph Hellwig
2 siblings, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2018-06-07 15:23 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Thu, Jun 07, 2018 at 03:27:49PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> New verification functions like xfs_verify_fsbno() and
> xfs_verify_agino() are spread across multiple files and different
> header files. They really don't fit cleanly into the places they've
> been put, and have wider scope than the current header includes.
>
> Move the type verifiers to a new file in libxfs (xfs-types.c) and
> the prototypes to xfs_types.h where they will be visible to all the
> code that uses the types.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/Makefile | 1 +
> fs/xfs/libxfs/xfs_alloc.c | 49 ----------
> fs/xfs/libxfs/xfs_alloc.h | 4 -
> fs/xfs/libxfs/xfs_ialloc.c | 90 ------------------
> fs/xfs/libxfs/xfs_ialloc.h | 7 --
> fs/xfs/libxfs/xfs_rtbitmap.c | 12 ---
> fs/xfs/libxfs/xfs_types.c | 173 +++++++++++++++++++++++++++++++++++
> fs/xfs/libxfs/xfs_types.h | 19 ++++
> fs/xfs/scrub/agheader.c | 2 +-
> 9 files changed, 194 insertions(+), 163 deletions(-)
> create mode 100644 fs/xfs/libxfs/xfs_types.c
>
> diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
> index 19a1f8064d8a..5333f7cbc95a 100644
> --- a/fs/xfs/Makefile
> +++ b/fs/xfs/Makefile
> @@ -50,6 +50,7 @@ xfs-y += $(addprefix libxfs/, \
> xfs_sb.o \
> xfs_symlink_remote.o \
> xfs_trans_resv.o \
> + xfs_types.o \
> )
> # xfs_rtbitmap is shared with libxfs
> xfs-$(CONFIG_XFS_RT) += $(addprefix libxfs/, \
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 1db50cfc0212..eef466260d43 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -3123,55 +3123,6 @@ xfs_alloc_query_all(
> return xfs_btree_query_all(cur, xfs_alloc_query_range_helper, &query);
> }
>
> -/* Find the size of the AG, in blocks. */
> -xfs_agblock_t
> -xfs_ag_block_count(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno)
> -{
> - ASSERT(agno < mp->m_sb.sb_agcount);
> -
> - if (agno < mp->m_sb.sb_agcount - 1)
> - return mp->m_sb.sb_agblocks;
> - return mp->m_sb.sb_dblocks - (agno * mp->m_sb.sb_agblocks);
> -}
> -
> -/*
> - * Verify that an AG block number pointer neither points outside the AG
> - * nor points at static metadata.
> - */
> -bool
> -xfs_verify_agbno(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno,
> - xfs_agblock_t agbno)
> -{
> - xfs_agblock_t eoag;
> -
> - eoag = xfs_ag_block_count(mp, agno);
> - if (agbno >= eoag)
> - return false;
> - if (agbno <= XFS_AGFL_BLOCK(mp))
> - return false;
> - return true;
> -}
> -
> -/*
> - * Verify that an FS block number pointer neither points outside the
> - * filesystem nor points at static AG metadata.
> - */
> -bool
> -xfs_verify_fsbno(
> - struct xfs_mount *mp,
> - xfs_fsblock_t fsbno)
> -{
> - xfs_agnumber_t agno = XFS_FSB_TO_AGNO(mp, fsbno);
> -
> - if (agno >= mp->m_sb.sb_agcount)
> - return false;
> - return xfs_verify_agbno(mp, agno, XFS_FSB_TO_AGBNO(mp, fsbno));
> -}
> -
> /* Is there a record covering a given extent? */
> int
> xfs_alloc_has_record(
> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
> index 1651d924aaf1..e716c993ac4c 100644
> --- a/fs/xfs/libxfs/xfs_alloc.h
> +++ b/fs/xfs/libxfs/xfs_alloc.h
> @@ -242,10 +242,6 @@ int xfs_alloc_query_range(struct xfs_btree_cur *cur,
> xfs_alloc_query_range_fn fn, void *priv);
> int xfs_alloc_query_all(struct xfs_btree_cur *cur, xfs_alloc_query_range_fn fn,
> void *priv);
> -xfs_agblock_t xfs_ag_block_count(struct xfs_mount *mp, xfs_agnumber_t agno);
> -bool xfs_verify_agbno(struct xfs_mount *mp, xfs_agnumber_t agno,
> - xfs_agblock_t agbno);
> -bool xfs_verify_fsbno(struct xfs_mount *mp, xfs_fsblock_t fsbno);
>
> int xfs_alloc_has_record(struct xfs_btree_cur *cur, xfs_agblock_t bno,
> xfs_extlen_t len, bool *exist);
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 3f551eb29157..8ec39dad62d7 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -2674,96 +2674,6 @@ xfs_ialloc_pagi_init(
> return 0;
> }
>
> -/* Calculate the first and last possible inode number in an AG. */
> -void
> -xfs_ialloc_agino_range(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno,
> - xfs_agino_t *first,
> - xfs_agino_t *last)
> -{
> - xfs_agblock_t bno;
> - xfs_agblock_t eoag;
> -
> - eoag = xfs_ag_block_count(mp, agno);
> -
> - /*
> - * Calculate the first inode, which will be in the first
> - * cluster-aligned block after the AGFL.
> - */
> - bno = round_up(XFS_AGFL_BLOCK(mp) + 1,
> - xfs_ialloc_cluster_alignment(mp));
> - *first = XFS_OFFBNO_TO_AGINO(mp, bno, 0);
> -
> - /*
> - * Calculate the last inode, which will be at the end of the
> - * last (aligned) cluster that can be allocated in the AG.
> - */
> - bno = round_down(eoag, xfs_ialloc_cluster_alignment(mp));
> - *last = XFS_OFFBNO_TO_AGINO(mp, bno, 0) - 1;
> -}
> -
> -/*
> - * Verify that an AG inode number pointer neither points outside the AG
> - * nor points at static metadata.
> - */
> -bool
> -xfs_verify_agino(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno,
> - xfs_agino_t agino)
> -{
> - xfs_agino_t first;
> - xfs_agino_t last;
> -
> - xfs_ialloc_agino_range(mp, agno, &first, &last);
> - return agino >= first && agino <= last;
> -}
> -
> -/*
> - * Verify that an FS inode number pointer neither points outside the
> - * filesystem nor points at static AG metadata.
> - */
> -bool
> -xfs_verify_ino(
> - struct xfs_mount *mp,
> - xfs_ino_t ino)
> -{
> - xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, ino);
> - xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ino);
> -
> - if (agno >= mp->m_sb.sb_agcount)
> - return false;
> - if (XFS_AGINO_TO_INO(mp, agno, agino) != ino)
> - return false;
> - return xfs_verify_agino(mp, agno, agino);
> -}
> -
> -/* Is this an internal inode number? */
> -bool
> -xfs_internal_inum(
> - struct xfs_mount *mp,
> - xfs_ino_t ino)
> -{
> - return ino == mp->m_sb.sb_rbmino || ino == mp->m_sb.sb_rsumino ||
> - (xfs_sb_version_hasquota(&mp->m_sb) &&
> - xfs_is_quota_inode(&mp->m_sb, ino));
> -}
> -
> -/*
> - * Verify that a directory entry's inode number doesn't point at an internal
> - * inode, empty space, or static AG metadata.
> - */
> -bool
> -xfs_verify_dir_ino(
> - struct xfs_mount *mp,
> - xfs_ino_t ino)
> -{
> - if (xfs_internal_inum(mp, ino))
> - return false;
> - return xfs_verify_ino(mp, ino);
> -}
> -
> /* Is there an inode record covering a given range of inode numbers? */
> int
> xfs_ialloc_has_inode_record(
> diff --git a/fs/xfs/libxfs/xfs_ialloc.h b/fs/xfs/libxfs/xfs_ialloc.h
> index 144f3eac9b93..90b09c5f163b 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.h
> +++ b/fs/xfs/libxfs/xfs_ialloc.h
> @@ -169,12 +169,5 @@ int xfs_inobt_insert_rec(struct xfs_btree_cur *cur, uint16_t holemask,
> int *stat);
>
> int xfs_ialloc_cluster_alignment(struct xfs_mount *mp);
> -void xfs_ialloc_agino_range(struct xfs_mount *mp, xfs_agnumber_t agno,
> - xfs_agino_t *first, xfs_agino_t *last);
> -bool xfs_verify_agino(struct xfs_mount *mp, xfs_agnumber_t agno,
> - xfs_agino_t agino);
> -bool xfs_verify_ino(struct xfs_mount *mp, xfs_ino_t ino);
> -bool xfs_internal_inum(struct xfs_mount *mp, xfs_ino_t ino);
> -bool xfs_verify_dir_ino(struct xfs_mount *mp, xfs_ino_t ino);
>
> #endif /* __XFS_IALLOC_H__ */
> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
> index ffc72075a44e..65fc4ed2e9a1 100644
> --- a/fs/xfs/libxfs/xfs_rtbitmap.c
> +++ b/fs/xfs/libxfs/xfs_rtbitmap.c
> @@ -1080,18 +1080,6 @@ xfs_rtalloc_query_all(
> return xfs_rtalloc_query_range(tp, &keys[0], &keys[1], fn, priv);
> }
>
> -/*
> - * Verify that an realtime block number pointer doesn't point off the
> - * end of the realtime device.
> - */
> -bool
> -xfs_verify_rtbno(
> - struct xfs_mount *mp,
> - xfs_rtblock_t rtbno)
> -{
> - return rtbno < mp->m_sb.sb_rblocks;
> -}
> -
> /* Is the given extent all free? */
> int
> xfs_rtalloc_extent_is_free(
> diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
> new file mode 100644
> index 000000000000..2e2a243cef2e
> --- /dev/null
> +++ b/fs/xfs/libxfs/xfs_types.c
> @@ -0,0 +1,173 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2000-2002,2005 Silicon Graphics, Inc.
> + * Copyright (C) 2017 Oracle.
> + * All Rights Reserved.
> + */
> +#include "xfs.h"
> +#include "xfs_fs.h"
> +#include "xfs_format.h"
> +#include "xfs_log_format.h"
> +#include "xfs_shared.h"
> +#include "xfs_trans_resv.h"
> +#include "xfs_bit.h"
> +#include "xfs_sb.h"
> +#include "xfs_mount.h"
> +#include "xfs_defer.h"
> +#include "xfs_inode.h"
> +#include "xfs_btree.h"
> +#include "xfs_rmap.h"
> +#include "xfs_alloc_btree.h"
> +#include "xfs_alloc.h"
> +#include "xfs_ialloc.h"
> +
> +/* Find the size of the AG, in blocks. */
> +xfs_agblock_t
> +xfs_ag_block_count(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno)
> +{
> + ASSERT(agno < mp->m_sb.sb_agcount);
> +
> + if (agno < mp->m_sb.sb_agcount - 1)
> + return mp->m_sb.sb_agblocks;
> + return mp->m_sb.sb_dblocks - (agno * mp->m_sb.sb_agblocks);
> +}
> +
> +/*
> + * Verify that an AG block number pointer neither points outside the AG
> + * nor points at static metadata.
> + */
> +bool
> +xfs_verify_agbno(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno,
> + xfs_agblock_t agbno)
> +{
> + xfs_agblock_t eoag;
> +
> + eoag = xfs_ag_block_count(mp, agno);
> + if (agbno >= eoag)
> + return false;
> + if (agbno <= XFS_AGFL_BLOCK(mp))
> + return false;
> + return true;
> +}
> +
> +/*
> + * Verify that an FS block number pointer neither points outside the
> + * filesystem nor points at static AG metadata.
> + */
> +bool
> +xfs_verify_fsbno(
> + struct xfs_mount *mp,
> + xfs_fsblock_t fsbno)
> +{
> + xfs_agnumber_t agno = XFS_FSB_TO_AGNO(mp, fsbno);
> +
> + if (agno >= mp->m_sb.sb_agcount)
> + return false;
> + return xfs_verify_agbno(mp, agno, XFS_FSB_TO_AGBNO(mp, fsbno));
> +}
> +
> +/* Calculate the first and last possible inode number in an AG. */
> +void
> +xfs_agino_range(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno,
> + xfs_agino_t *first,
> + xfs_agino_t *last)
> +{
> + xfs_agblock_t bno;
> + xfs_agblock_t eoag;
> +
> + eoag = xfs_ag_block_count(mp, agno);
> +
> + /*
> + * Calculate the first inode, which will be in the first
> + * cluster-aligned block after the AGFL.
> + */
> + bno = round_up(XFS_AGFL_BLOCK(mp) + 1,
> + xfs_ialloc_cluster_alignment(mp));
> + *first = XFS_OFFBNO_TO_AGINO(mp, bno, 0);
> +
> + /*
> + * Calculate the last inode, which will be at the end of the
> + * last (aligned) cluster that can be allocated in the AG.
> + */
> + bno = round_down(eoag, xfs_ialloc_cluster_alignment(mp));
> + *last = XFS_OFFBNO_TO_AGINO(mp, bno, 0) - 1;
> +}
> +
> +/*
> + * Verify that an AG inode number pointer neither points outside the AG
> + * nor points at static metadata.
> + */
> +bool
> +xfs_verify_agino(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno,
> + xfs_agino_t agino)
> +{
> + xfs_agino_t first;
> + xfs_agino_t last;
> +
> + xfs_agino_range(mp, agno, &first, &last);
> + return agino >= first && agino <= last;
> +}
> +
> +/*
> + * Verify that an FS inode number pointer neither points outside the
> + * filesystem nor points at static AG metadata.
> + */
> +bool
> +xfs_verify_ino(
> + struct xfs_mount *mp,
> + xfs_ino_t ino)
> +{
> + xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, ino);
> + xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ino);
> +
> + if (agno >= mp->m_sb.sb_agcount)
> + return false;
> + if (XFS_AGINO_TO_INO(mp, agno, agino) != ino)
> + return false;
> + return xfs_verify_agino(mp, agno, agino);
> +}
> +
> +/* Is this an internal inode number? */
> +bool
> +xfs_internal_inum(
> + struct xfs_mount *mp,
> + xfs_ino_t ino)
> +{
> + return ino == mp->m_sb.sb_rbmino || ino == mp->m_sb.sb_rsumino ||
> + (xfs_sb_version_hasquota(&mp->m_sb) &&
> + xfs_is_quota_inode(&mp->m_sb, ino));
> +}
> +
> +/*
> + * Verify that a directory entry's inode number doesn't point at an internal
> + * inode, empty space, or static AG metadata.
> + */
> +bool
> +xfs_verify_dir_ino(
> + struct xfs_mount *mp,
> + xfs_ino_t ino)
> +{
> + if (xfs_internal_inum(mp, ino))
> + return false;
> + return xfs_verify_ino(mp, ino);
> +}
> +
> +/*
> + * Verify that an realtime block number pointer doesn't point off the
> + * end of the realtime device.
> + */
> +bool
> +xfs_verify_rtbno(
> + struct xfs_mount *mp,
> + xfs_rtblock_t rtbno)
> +{
> + return rtbno < mp->m_sb.sb_rblocks;
> +}
> diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
> index b72ae343140e..4055d62f690c 100644
> --- a/fs/xfs/libxfs/xfs_types.h
> +++ b/fs/xfs/libxfs/xfs_types.h
> @@ -147,4 +147,23 @@ typedef struct xfs_bmbt_irec
> xfs_exntst_t br_state; /* extent state */
> } xfs_bmbt_irec_t;
>
> +/*
> + * Type verifier functions
> + */
> +struct xfs_mount;
> +
> +xfs_agblock_t xfs_ag_block_count(struct xfs_mount *mp, xfs_agnumber_t agno);
> +bool xfs_verify_agbno(struct xfs_mount *mp, xfs_agnumber_t agno,
> + xfs_agblock_t agbno);
> +bool xfs_verify_fsbno(struct xfs_mount *mp, xfs_fsblock_t fsbno);
> +
> +void xfs_agino_range(struct xfs_mount *mp, xfs_agnumber_t agno,
> + xfs_agino_t *first, xfs_agino_t *last);
> +bool xfs_verify_agino(struct xfs_mount *mp, xfs_agnumber_t agno,
> + xfs_agino_t agino);
> +bool xfs_verify_ino(struct xfs_mount *mp, xfs_ino_t ino);
> +bool xfs_internal_inum(struct xfs_mount *mp, xfs_ino_t ino);
> +bool xfs_verify_dir_ino(struct xfs_mount *mp, xfs_ino_t ino);
> +bool xfs_verify_rtbno(struct xfs_mount *mp, xfs_rtblock_t rtbno);
> +
> #endif /* __XFS_TYPES_H__ */
> diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
> index fb9637ff4bde..9bb0745f1ad2 100644
> --- a/fs/xfs/scrub/agheader.c
> +++ b/fs/xfs/scrub/agheader.c
> @@ -867,7 +867,7 @@ xfs_scrub_agi(
> }
>
> /* Check inode counters */
> - xfs_ialloc_agino_range(mp, agno, &first_agino, &last_agino);
> + xfs_agino_range(mp, agno, &first_agino, &last_agino);
> icount = be32_to_cpu(agi->agi_count);
> if (icount > last_agino - first_agino + 1 ||
> icount < be32_to_cpu(agi->agi_freecount))
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] xfs: replace do_mod with native operations
2018-06-07 5:27 ` [PATCH 2/3] xfs: replace do_mod with native operations Dave Chinner
2018-06-07 11:42 ` Brian Foster
@ 2018-06-07 15:54 ` Darrick J. Wong
2018-06-07 22:28 ` Dave Chinner
2018-06-08 0:43 ` [PATCH 2/3 V2] " Dave Chinner
2 siblings, 1 reply; 17+ messages in thread
From: Darrick J. Wong @ 2018-06-07 15:54 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Thu, Jun 07, 2018 at 03:27:50PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> do_mod() is a hold-over from when we have different sizes for file
> offsets and and other internal values for 40 bit XFS filesystems.
> Hence depending on build flags variables passed to do_mod() could
> change size. We no longer support those small format filesystems and
> hence everything is of fixed size theses days, even on 32 bit
> platforms.
>
> As such, we can convert all the do_mod() callers to platform
> optimised modulus operations as defined by linux/math64.h.
> Individual conversions depend on the types of variables being used.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 37 +++++++++++++++++++++++--------------
> fs/xfs/xfs_bmap_util.c | 14 +++++++++-----
> fs/xfs/xfs_inode.c | 2 +-
> fs/xfs/xfs_iomap.h | 4 ++--
> fs/xfs/xfs_linux.h | 19 -------------------
> fs/xfs/xfs_log_recover.c | 32 +++++++++++++++++++++++++-------
> fs/xfs/xfs_rtalloc.c | 15 +++++++++++----
> 7 files changed, 71 insertions(+), 52 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 3de047eb8209..4d8fd37ec7ae 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -2923,7 +2923,7 @@ xfs_bmap_extsize_align(
> * perform this alignment, or if a truncate shot us in the
> * foot.
> */
> - temp = do_mod(orig_off, extsz);
> + div_u64_rem(orig_off, extsz, &temp);
> if (temp) {
> align_alen += temp;
> align_off -= temp;
> @@ -3497,15 +3497,17 @@ xfs_bmap_btalloc(
> /* apply extent size hints if obtained earlier */
> if (align) {
> args.prod = align;
> - if ((args.mod = (xfs_extlen_t)do_mod(ap->offset, args.prod)))
> - args.mod = (xfs_extlen_t)(args.prod - args.mod);
> + div_u64_rem(ap->offset, args.prod, &args.mod);
> + if (args.mod)
> + args.mod = args.prod - args.mod;
> } else if (mp->m_sb.sb_blocksize >= PAGE_SIZE) {
> args.prod = 1;
> args.mod = 0;
> } else {
> args.prod = PAGE_SIZE >> mp->m_sb.sb_blocklog;
> - if ((args.mod = (xfs_extlen_t)(do_mod(ap->offset, args.prod))))
> - args.mod = (xfs_extlen_t)(args.prod - args.mod);
> + div_u64_rem(ap->offset, args.prod, &args.mod);
> + if (args.mod)
> + args.mod = args.prod - args.mod;
> }
> /*
> * If we are not low on available data blocks, and the
> @@ -4953,13 +4955,15 @@ xfs_bmap_del_extent_real(
> if (whichfork == XFS_DATA_FORK && XFS_IS_REALTIME_INODE(ip)) {
> xfs_fsblock_t bno;
> xfs_filblks_t len;
> + xfs_extlen_t mod;
> +
> + bno = div_u64_rem(del->br_startblock, mp->m_sb.sb_rextsize,
> + &mod);
> + ASSERT(mod == 0);
> + len = div_u64_rem(del->br_blockcount, mp->m_sb.sb_rextsize,
> + &mod);
> + ASSERT(mod == 0);
>
> - ASSERT(do_mod(del->br_blockcount, mp->m_sb.sb_rextsize) == 0);
> - ASSERT(do_mod(del->br_startblock, mp->m_sb.sb_rextsize) == 0);
> - bno = del->br_startblock;
> - len = del->br_blockcount;
> - do_div(bno, mp->m_sb.sb_rextsize);
> - do_div(len, mp->m_sb.sb_rextsize);
> error = xfs_rtfree_extent(tp, bno, (xfs_extlen_t)len);
> if (error)
> goto done;
> @@ -5296,9 +5300,12 @@ __xfs_bunmapi(
> del.br_blockcount = max_len;
> }
>
> + if (!isrt)
> + goto delete;
> +
> sum = del.br_startblock + del.br_blockcount;
> - if (isrt &&
> - (mod = do_mod(sum, mp->m_sb.sb_rextsize))) {
> + div_u64_rem(sum, mp->m_sb.sb_rextsize, &mod);
> + if (mod) {
> /*
> * Realtime extent not lined up at the end.
> * The extent could have been split into written
> @@ -5345,7 +5352,8 @@ __xfs_bunmapi(
> goto error0;
> goto nodelete;
> }
> - if (isrt && (mod = do_mod(del.br_startblock, mp->m_sb.sb_rextsize))) {
> + div_u64_rem(del.br_startblock, mp->m_sb.sb_rextsize, &mod);
> + if (mod) {
> /*
> * Realtime extent is lined up at the end but not
> * at the front. We'll get rid of full extents if
> @@ -5414,6 +5422,7 @@ __xfs_bunmapi(
> }
> }
>
> +delete:
> if (wasdel) {
> error = xfs_bmap_del_extent_delay(ip, whichfork, &icur,
> &got, &del);
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 7d26933a542f..13e6caf8b801 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -80,6 +80,7 @@ xfs_bmap_rtalloc(
> int error; /* error return value */
> xfs_mount_t *mp; /* mount point structure */
> xfs_extlen_t prod = 0; /* product factor for allocators */
> + xfs_extlen_t mod = 0; /* product factor for allocators */
I don't think we need the initial value or the comment.
> xfs_extlen_t ralen = 0; /* realtime allocation length */
> xfs_extlen_t align; /* minimum allocation alignment */
> xfs_rtblock_t rtb;
> @@ -99,7 +100,8 @@ xfs_bmap_rtalloc(
> * If the offset & length are not perfectly aligned
> * then kill prod, it will just get us in trouble.
> */
> - if (do_mod(ap->offset, align) || ap->length % align)
> + div_u64_rem(ap->offset, align, &mod);
> + if (mod || ap->length % align)
> prod = 1;
> /*
> * Set ralen to be the actual requested length in rtextents.
> @@ -936,9 +938,11 @@ xfs_alloc_file_space(
> do_div(s, extsz);
> s *= extsz;
> e = startoffset_fsb + allocatesize_fsb;
> - if ((temp = do_mod(startoffset_fsb, extsz)))
> + div_u64_rem(startoffset_fsb, extsz, &temp);
> + if (temp)
> e += temp;
> - if ((temp = do_mod(e, extsz)))
> + div_u64_rem(e, extsz, &temp);
> + if (temp)
> e += extsz - temp;
> } else {
> s = 0;
> @@ -1090,7 +1094,7 @@ xfs_adjust_extent_unmap_boundaries(
> struct xfs_mount *mp = ip->i_mount;
> struct xfs_bmbt_irec imap;
> int nimap, error;
> - xfs_extlen_t mod = 0;
> + xfs_rtblock_t mod = 0;
>
> nimap = 1;
> error = xfs_bmapi_read(ip, *startoffset_fsb, 1, &imap, &nimap, 0);
> @@ -1099,7 +1103,7 @@ xfs_adjust_extent_unmap_boundaries(
>
> if (nimap && imap.br_startblock != HOLESTARTBLOCK) {
> ASSERT(imap.br_startblock != DELAYSTARTBLOCK);
> - mod = do_mod(imap.br_startblock, mp->m_sb.sb_rextsize);
> + div64_u64_rem(imap.br_startblock, mp->m_sb.sb_rextsize, &mod);
Why does this need to be a div64_u64_rem? sb_rextsize is 32-bit, so the
remainder won't exceed 2^32-1, right?
> if (mod)
> *startoffset_fsb += mp->m_sb.sb_rextsize - mod;
Indeed if it ever did that would screw up this logic, wouldn't it?
--D
> }
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 6cda0f08b045..4a2e5e13c569 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2258,7 +2258,7 @@ xfs_ifree_cluster(
> */
> ioffset = inum - xic->first_ino;
> if ((xic->alloc & XFS_INOBT_MASK(ioffset)) == 0) {
> - ASSERT(do_mod(ioffset, inodes_per_cluster) == 0);
> + ASSERT(ioffset % inodes_per_cluster == 0);
> continue;
> }
>
> diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
> index b0c98d4faa5b..83474c9cede9 100644
> --- a/fs/xfs/xfs_iomap.h
> +++ b/fs/xfs/xfs_iomap.h
> @@ -30,10 +30,10 @@ xfs_aligned_fsb_count(
> if (extsz) {
> xfs_extlen_t align;
>
> - align = do_mod(offset_fsb, extsz);
> + div_u64_rem(offset_fsb, extsz, &align);
> if (align)
> count_fsb += align;
> - align = do_mod(count_fsb, extsz);
> + div_u64_rem(count_fsb, extsz, &align);
> if (align)
> count_fsb += extsz - align;
> }
> diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
> index 1631cf4546f2..2c800ffbcffd 100644
> --- a/fs/xfs/xfs_linux.h
> +++ b/fs/xfs/xfs_linux.h
> @@ -209,25 +209,6 @@ static inline xfs_dev_t linux_to_xfs_dev_t(dev_t dev)
> #define xfs_sort(a,n,s,fn) sort(a,n,s,fn,NULL)
> #define xfs_stack_trace() dump_stack()
>
> -/* Side effect free 64 bit mod operation */
> -static inline __u32 xfs_do_mod(void *a, __u32 b, int n)
> -{
> - switch (n) {
> - case 4:
> - return *(__u32 *)a % b;
> - case 8:
> - {
> - __u64 c = *(__u64 *)a;
> - return do_div(c, b);
> - }
> - }
> -
> - /* NOTREACHED */
> - return 0;
> -}
> -
> -#define do_mod(a, b) xfs_do_mod(&(a), (b), sizeof(a))
> -
> static inline uint64_t roundup_64(uint64_t x, uint32_t y)
> {
> x += y - 1;
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 7d897c58b0c8..4405ff21f9a9 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -1235,6 +1235,25 @@ xlog_verify_head(
> be32_to_cpu((*rhead)->h_size));
> }
>
> +/*
> + * We need to make sure we handle log wrapping properly, so we can't use teh
> + * calculated logbno directly. Make sure it wraps to teh correct bno inside teh
> + * log.
> + *
> + * The log is limited to 32 bit sizes, so we use the appropriate modulus
> + * operation here and cast it back to a 64 bit daddr on return.
> + */
> +static inline xfs_daddr_t
> +xlog_wrap_logbno(
> + struct xlog *log,
> + xfs_daddr_t bno)
> +{
> + int mod;
> +
> + div_s64_rem(bno, log->l_logBBsize, &mod);
> + return mod;
> +}
> +
> /*
> * Check whether the head of the log points to an unmount record. In other
> * words, determine whether the log is clean. If so, update the in-core state
> @@ -1283,12 +1302,13 @@ xlog_check_unmount_rec(
> } else {
> hblks = 1;
> }
> - after_umount_blk = rhead_blk + hblks + BTOBB(be32_to_cpu(rhead->h_len));
> - after_umount_blk = do_mod(after_umount_blk, log->l_logBBsize);
> +
> + after_umount_blk = xlog_wrap_logbno(log,
> + rhead_blk + hblks + BTOBB(be32_to_cpu(rhead->h_len)));
> +
> if (*head_blk == after_umount_blk &&
> be32_to_cpu(rhead->h_num_logops) == 1) {
> - umount_data_blk = rhead_blk + hblks;
> - umount_data_blk = do_mod(umount_data_blk, log->l_logBBsize);
> + umount_data_blk = xlog_wrap_logbno(log, rhead_blk + hblks);
> error = xlog_bread(log, umount_data_blk, 1, bp, &offset);
> if (error)
> return error;
> @@ -5459,9 +5479,7 @@ xlog_do_recovery_pass(
> */
> if (blk_no + bblks <= log->l_logBBsize ||
> blk_no >= log->l_logBBsize) {
> - /* mod blk_no in case the header wrapped and
> - * pushed it beyond the end of the log */
> - rblk_no = do_mod(blk_no, log->l_logBBsize);
> + rblk_no = xlog_wrap_logbno(log, blk_no);
> error = xlog_bread(log, rblk_no, bblks, dbp,
> &offset);
> if (error)
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 80bbfe604ce0..776502a5dcb7 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -301,8 +301,12 @@ xfs_rtallocate_extent_block(
> /*
> * If size should be a multiple of prod, make that so.
> */
> - if (prod > 1 && (p = do_mod(bestlen, prod)))
> - bestlen -= p;
> + if (prod > 1) {
> + div_u64_rem(bestlen, prod, &p);
> + if (p)
> + bestlen -= p;
> + }
> +
> /*
> * Allocate besti for bestlen & return that.
> */
> @@ -1262,8 +1266,11 @@ xfs_rtpick_extent(
> resid = seq - (1ULL << log2);
> b = (mp->m_sb.sb_rextents * ((resid << 1) + 1ULL)) >>
> (log2 + 1);
> - if (b >= mp->m_sb.sb_rextents)
> - b = do_mod(b, mp->m_sb.sb_rextents);
> + if (b >= mp->m_sb.sb_rextents) {
> + xfs_rtblock_t mod;
> + div64_u64_rem(b, mp->m_sb.sb_rextents, &mod);
> + b = mod;
> + }
> if (b + len > mp->m_sb.sb_rextents)
> b = mp->m_sb.sb_rextents - len;
> }
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] xfs: replace do_mod with native operations
2018-06-07 11:42 ` Brian Foster
@ 2018-06-07 16:01 ` Darrick J. Wong
2018-06-07 22:23 ` Dave Chinner
1 sibling, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2018-06-07 16:01 UTC (permalink / raw)
To: Brian Foster; +Cc: Dave Chinner, linux-xfs
On Thu, Jun 07, 2018 at 07:42:05AM -0400, Brian Foster wrote:
> On Thu, Jun 07, 2018 at 03:27:50PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > do_mod() is a hold-over from when we have different sizes for file
> > offsets and and other internal values for 40 bit XFS filesystems.
> > Hence depending on build flags variables passed to do_mod() could
> > change size. We no longer support those small format filesystems and
> > hence everything is of fixed size theses days, even on 32 bit
> > platforms.
> >
> > As such, we can convert all the do_mod() callers to platform
> > optimised modulus operations as defined by linux/math64.h.
> > Individual conversions depend on the types of variables being used.
> >
> > Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> > ---
> > fs/xfs/libxfs/xfs_bmap.c | 37 +++++++++++++++++++++++--------------
> > fs/xfs/xfs_bmap_util.c | 14 +++++++++-----
> > fs/xfs/xfs_inode.c | 2 +-
> > fs/xfs/xfs_iomap.h | 4 ++--
> > fs/xfs/xfs_linux.h | 19 -------------------
> > fs/xfs/xfs_log_recover.c | 32 +++++++++++++++++++++++++-------
> > fs/xfs/xfs_rtalloc.c | 15 +++++++++++----
> > 7 files changed, 71 insertions(+), 52 deletions(-)
> >
> ...
> > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> > index 7d897c58b0c8..4405ff21f9a9 100644
> > --- a/fs/xfs/xfs_log_recover.c
> > +++ b/fs/xfs/xfs_log_recover.c
> > @@ -1235,6 +1235,25 @@ xlog_verify_head(
> > be32_to_cpu((*rhead)->h_size));
> > }
> >
> > +/*
> > + * We need to make sure we handle log wrapping properly, so we can't use teh
> > + * calculated logbno directly. Make sure it wraps to teh correct bno inside teh
> > + * log.
> > + *
>
> s/teh/the/
>
> > + * The log is limited to 32 bit sizes, so we use the appropriate modulus
> > + * operation here and cast it back to a 64 bit daddr on return.
> > + */
> > +static inline xfs_daddr_t
> > +xlog_wrap_logbno(
> > + struct xlog *log,
> > + xfs_daddr_t bno)
> > +{
> > + int mod;
> > +
> > + div_s64_rem(bno, log->l_logBBsize, &mod);
> > + return mod;
> > +}
> > +
> > /*
> > * Check whether the head of the log points to an unmount record. In other
> > * words, determine whether the log is clean. If so, update the in-core state
> ...
> > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> > index 80bbfe604ce0..776502a5dcb7 100644
> > --- a/fs/xfs/xfs_rtalloc.c
> > +++ b/fs/xfs/xfs_rtalloc.c
> ...
> > @@ -1262,8 +1266,11 @@ xfs_rtpick_extent(
> > resid = seq - (1ULL << log2);
> > b = (mp->m_sb.sb_rextents * ((resid << 1) + 1ULL)) >>
> > (log2 + 1);
> > - if (b >= mp->m_sb.sb_rextents)
> > - b = do_mod(b, mp->m_sb.sb_rextents);
> > + if (b >= mp->m_sb.sb_rextents) {
> > + xfs_rtblock_t mod;
> > + div64_u64_rem(b, mp->m_sb.sb_rextents, &mod);
> > + b = mod;
> > + }
>
> Shouldn't we be able to do 'div64_u64_rem(b, mp->m_sb.sb_rextents, &b)'
> here? Otherwise looks fine:
I think Dave is trying to avoid aliasing the arguments in case
div64_u64_rem ever gets turned into a horrible macro (or over-optimized
by gcc)
--D
>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
>
> > if (b + len > mp->m_sb.sb_rextents)
> > b = mp->m_sb.sb_rextents - len;
> > }
> > --
> > 2.17.0
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] xfs: replace do_mod with native operations
2018-06-07 11:42 ` Brian Foster
2018-06-07 16:01 ` Darrick J. Wong
@ 2018-06-07 22:23 ` Dave Chinner
1 sibling, 0 replies; 17+ messages in thread
From: Dave Chinner @ 2018-06-07 22:23 UTC (permalink / raw)
To: Brian Foster; +Cc: linux-xfs
On Thu, Jun 07, 2018 at 07:42:05AM -0400, Brian Foster wrote:
> On Thu, Jun 07, 2018 at 03:27:50PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > do_mod() is a hold-over from when we have different sizes for file
> > offsets and and other internal values for 40 bit XFS filesystems.
> > Hence depending on build flags variables passed to do_mod() could
> > change size. We no longer support those small format filesystems and
> > hence everything is of fixed size theses days, even on 32 bit
> > platforms.
> >
> > As such, we can convert all the do_mod() callers to platform
> > optimised modulus operations as defined by linux/math64.h.
> > Individual conversions depend on the types of variables being used.
> >
> > Signed-Off-By: Dave Chinner <dchinner@redhat.com>
....
> > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> > index 80bbfe604ce0..776502a5dcb7 100644
> > --- a/fs/xfs/xfs_rtalloc.c
> > +++ b/fs/xfs/xfs_rtalloc.c
> ...
> > @@ -1262,8 +1266,11 @@ xfs_rtpick_extent(
> > resid = seq - (1ULL << log2);
> > b = (mp->m_sb.sb_rextents * ((resid << 1) + 1ULL)) >>
> > (log2 + 1);
> > - if (b >= mp->m_sb.sb_rextents)
> > - b = do_mod(b, mp->m_sb.sb_rextents);
> > + if (b >= mp->m_sb.sb_rextents) {
> > + xfs_rtblock_t mod;
> > + div64_u64_rem(b, mp->m_sb.sb_rextents, &mod);
> > + b = mod;
> > + }
>
> Shouldn't we be able to do 'div64_u64_rem(b, mp->m_sb.sb_rextents, &b)'
> here? Otherwise looks fine:
Yeah, I think we can. IIRC I was looking at various implementations
and took the template from something like dm_sector_div64() because
"obviously correct" :)
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] xfs: replace do_mod with native operations
2018-06-07 15:54 ` Darrick J. Wong
@ 2018-06-07 22:28 ` Dave Chinner
0 siblings, 0 replies; 17+ messages in thread
From: Dave Chinner @ 2018-06-07 22:28 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Thu, Jun 07, 2018 at 08:54:37AM -0700, Darrick J. Wong wrote:
> On Thu, Jun 07, 2018 at 03:27:50PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > do_mod() is a hold-over from when we have different sizes for file
> > offsets and and other internal values for 40 bit XFS filesystems.
> > Hence depending on build flags variables passed to do_mod() could
> > change size. We no longer support those small format filesystems and
> > hence everything is of fixed size theses days, even on 32 bit
> > platforms.
> >
> > As such, we can convert all the do_mod() callers to platform
> > optimised modulus operations as defined by linux/math64.h.
> > Individual conversions depend on the types of variables being used.
> >
> > Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> > ---
.....
> > @@ -1090,7 +1094,7 @@ xfs_adjust_extent_unmap_boundaries(
> > struct xfs_mount *mp = ip->i_mount;
> > struct xfs_bmbt_irec imap;
> > int nimap, error;
> > - xfs_extlen_t mod = 0;
> > + xfs_rtblock_t mod = 0;
> >
> > nimap = 1;
> > error = xfs_bmapi_read(ip, *startoffset_fsb, 1, &imap, &nimap, 0);
> > @@ -1099,7 +1103,7 @@ xfs_adjust_extent_unmap_boundaries(
> >
> > if (nimap && imap.br_startblock != HOLESTARTBLOCK) {
> > ASSERT(imap.br_startblock != DELAYSTARTBLOCK);
> > - mod = do_mod(imap.br_startblock, mp->m_sb.sb_rextsize);
> > + div64_u64_rem(imap.br_startblock, mp->m_sb.sb_rextsize, &mod);
>
> Why does this need to be a div64_u64_rem? sb_rextsize is 32-bit, so the
> remainder won't exceed 2^32-1, right?
>
> > if (mod)
> > *startoffset_fsb += mp->m_sb.sb_rextsize - mod;
>
> Indeed if it ever did that would screw up this logic, wouldn't it?
Yeah, i think I screwed that one up, probably confused rextsize with
rextents. IOWs mod can remain as a xfs_extlen_t and this can become
div_u64_rem().
I'll fix and repost.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/3 V2] xfs: replace do_mod with native operations
2018-06-07 5:27 ` [PATCH 2/3] xfs: replace do_mod with native operations Dave Chinner
2018-06-07 11:42 ` Brian Foster
2018-06-07 15:54 ` Darrick J. Wong
@ 2018-06-08 0:43 ` Dave Chinner
2018-06-08 6:19 ` Christoph Hellwig
2 siblings, 1 reply; 17+ messages in thread
From: Dave Chinner @ 2018-06-08 0:43 UTC (permalink / raw)
To: linux-xfs
From: Dave Chinner <dchinner@redhat.com>
do_mod() is a hold-over from when we have different sizes for file
offsets and and other internal values for 40 bit XFS filesystems.
Hence depending on build flags variables passed to do_mod() could
change size. We no longer support those small format filesystems and
hence everything is of fixed size theses days, even on 32 bit
platforms.
As such, we can convert all the do_mod() callers to platform
optimised modulus operations as defined by linux/math64.h.
Individual conversions depend on the types of variables being used.
Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
Version 2:
- fix typos
- remove unnecessary temp var from xfs_rtextent_pick
- use div_u64_rem() in xfs_adjust_extent_unmap_boundaries()
fs/xfs/libxfs/xfs_bmap.c | 37 +++++++++++++++++++++++--------------
fs/xfs/xfs_bmap_util.c | 12 ++++++++----
fs/xfs/xfs_inode.c | 2 +-
fs/xfs/xfs_iomap.h | 4 ++--
fs/xfs/xfs_linux.h | 19 -------------------
fs/xfs/xfs_log_recover.c | 32 +++++++++++++++++++++++++-------
fs/xfs/xfs_rtalloc.c | 10 +++++++---
7 files changed, 66 insertions(+), 50 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 3de047eb8209..4d8fd37ec7ae 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -2923,7 +2923,7 @@ xfs_bmap_extsize_align(
* perform this alignment, or if a truncate shot us in the
* foot.
*/
- temp = do_mod(orig_off, extsz);
+ div_u64_rem(orig_off, extsz, &temp);
if (temp) {
align_alen += temp;
align_off -= temp;
@@ -3497,15 +3497,17 @@ xfs_bmap_btalloc(
/* apply extent size hints if obtained earlier */
if (align) {
args.prod = align;
- if ((args.mod = (xfs_extlen_t)do_mod(ap->offset, args.prod)))
- args.mod = (xfs_extlen_t)(args.prod - args.mod);
+ div_u64_rem(ap->offset, args.prod, &args.mod);
+ if (args.mod)
+ args.mod = args.prod - args.mod;
} else if (mp->m_sb.sb_blocksize >= PAGE_SIZE) {
args.prod = 1;
args.mod = 0;
} else {
args.prod = PAGE_SIZE >> mp->m_sb.sb_blocklog;
- if ((args.mod = (xfs_extlen_t)(do_mod(ap->offset, args.prod))))
- args.mod = (xfs_extlen_t)(args.prod - args.mod);
+ div_u64_rem(ap->offset, args.prod, &args.mod);
+ if (args.mod)
+ args.mod = args.prod - args.mod;
}
/*
* If we are not low on available data blocks, and the
@@ -4953,13 +4955,15 @@ xfs_bmap_del_extent_real(
if (whichfork == XFS_DATA_FORK && XFS_IS_REALTIME_INODE(ip)) {
xfs_fsblock_t bno;
xfs_filblks_t len;
+ xfs_extlen_t mod;
+
+ bno = div_u64_rem(del->br_startblock, mp->m_sb.sb_rextsize,
+ &mod);
+ ASSERT(mod == 0);
+ len = div_u64_rem(del->br_blockcount, mp->m_sb.sb_rextsize,
+ &mod);
+ ASSERT(mod == 0);
- ASSERT(do_mod(del->br_blockcount, mp->m_sb.sb_rextsize) == 0);
- ASSERT(do_mod(del->br_startblock, mp->m_sb.sb_rextsize) == 0);
- bno = del->br_startblock;
- len = del->br_blockcount;
- do_div(bno, mp->m_sb.sb_rextsize);
- do_div(len, mp->m_sb.sb_rextsize);
error = xfs_rtfree_extent(tp, bno, (xfs_extlen_t)len);
if (error)
goto done;
@@ -5296,9 +5300,12 @@ __xfs_bunmapi(
del.br_blockcount = max_len;
}
+ if (!isrt)
+ goto delete;
+
sum = del.br_startblock + del.br_blockcount;
- if (isrt &&
- (mod = do_mod(sum, mp->m_sb.sb_rextsize))) {
+ div_u64_rem(sum, mp->m_sb.sb_rextsize, &mod);
+ if (mod) {
/*
* Realtime extent not lined up at the end.
* The extent could have been split into written
@@ -5345,7 +5352,8 @@ __xfs_bunmapi(
goto error0;
goto nodelete;
}
- if (isrt && (mod = do_mod(del.br_startblock, mp->m_sb.sb_rextsize))) {
+ div_u64_rem(del.br_startblock, mp->m_sb.sb_rextsize, &mod);
+ if (mod) {
/*
* Realtime extent is lined up at the end but not
* at the front. We'll get rid of full extents if
@@ -5414,6 +5422,7 @@ __xfs_bunmapi(
}
}
+delete:
if (wasdel) {
error = xfs_bmap_del_extent_delay(ip, whichfork, &icur,
&got, &del);
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 7d26933a542f..c35009a86699 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -80,6 +80,7 @@ xfs_bmap_rtalloc(
int error; /* error return value */
xfs_mount_t *mp; /* mount point structure */
xfs_extlen_t prod = 0; /* product factor for allocators */
+ xfs_extlen_t mod = 0; /* product factor for allocators */
xfs_extlen_t ralen = 0; /* realtime allocation length */
xfs_extlen_t align; /* minimum allocation alignment */
xfs_rtblock_t rtb;
@@ -99,7 +100,8 @@ xfs_bmap_rtalloc(
* If the offset & length are not perfectly aligned
* then kill prod, it will just get us in trouble.
*/
- if (do_mod(ap->offset, align) || ap->length % align)
+ div_u64_rem(ap->offset, align, &mod);
+ if (mod || ap->length % align)
prod = 1;
/*
* Set ralen to be the actual requested length in rtextents.
@@ -936,9 +938,11 @@ xfs_alloc_file_space(
do_div(s, extsz);
s *= extsz;
e = startoffset_fsb + allocatesize_fsb;
- if ((temp = do_mod(startoffset_fsb, extsz)))
+ div_u64_rem(startoffset_fsb, extsz, &temp);
+ if (temp)
e += temp;
- if ((temp = do_mod(e, extsz)))
+ div_u64_rem(e, extsz, &temp);
+ if (temp)
e += extsz - temp;
} else {
s = 0;
@@ -1099,7 +1103,7 @@ xfs_adjust_extent_unmap_boundaries(
if (nimap && imap.br_startblock != HOLESTARTBLOCK) {
ASSERT(imap.br_startblock != DELAYSTARTBLOCK);
- mod = do_mod(imap.br_startblock, mp->m_sb.sb_rextsize);
+ div_u64_rem(imap.br_startblock, mp->m_sb.sb_rextsize, &mod);
if (mod)
*startoffset_fsb += mp->m_sb.sb_rextsize - mod;
}
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 6cda0f08b045..4a2e5e13c569 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2258,7 +2258,7 @@ xfs_ifree_cluster(
*/
ioffset = inum - xic->first_ino;
if ((xic->alloc & XFS_INOBT_MASK(ioffset)) == 0) {
- ASSERT(do_mod(ioffset, inodes_per_cluster) == 0);
+ ASSERT(ioffset % inodes_per_cluster == 0);
continue;
}
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index b0c98d4faa5b..83474c9cede9 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -30,10 +30,10 @@ xfs_aligned_fsb_count(
if (extsz) {
xfs_extlen_t align;
- align = do_mod(offset_fsb, extsz);
+ div_u64_rem(offset_fsb, extsz, &align);
if (align)
count_fsb += align;
- align = do_mod(count_fsb, extsz);
+ div_u64_rem(count_fsb, extsz, &align);
if (align)
count_fsb += extsz - align;
}
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index 1631cf4546f2..2c800ffbcffd 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -209,25 +209,6 @@ static inline xfs_dev_t linux_to_xfs_dev_t(dev_t dev)
#define xfs_sort(a,n,s,fn) sort(a,n,s,fn,NULL)
#define xfs_stack_trace() dump_stack()
-/* Side effect free 64 bit mod operation */
-static inline __u32 xfs_do_mod(void *a, __u32 b, int n)
-{
- switch (n) {
- case 4:
- return *(__u32 *)a % b;
- case 8:
- {
- __u64 c = *(__u64 *)a;
- return do_div(c, b);
- }
- }
-
- /* NOTREACHED */
- return 0;
-}
-
-#define do_mod(a, b) xfs_do_mod(&(a), (b), sizeof(a))
-
static inline uint64_t roundup_64(uint64_t x, uint32_t y)
{
x += y - 1;
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 7d897c58b0c8..22f546eb421a 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -1235,6 +1235,25 @@ xlog_verify_head(
be32_to_cpu((*rhead)->h_size));
}
+/*
+ * We need to make sure we handle log wrapping properly, so we can't use the
+ * calculated logbno directly. Make sure it wraps to the correct bno inside the
+ * log.
+ *
+ * The log is limited to 32 bit sizes, so we use the appropriate modulus
+ * operation here and cast it back to a 64 bit daddr on return.
+ */
+static inline xfs_daddr_t
+xlog_wrap_logbno(
+ struct xlog *log,
+ xfs_daddr_t bno)
+{
+ int mod;
+
+ div_s64_rem(bno, log->l_logBBsize, &mod);
+ return mod;
+}
+
/*
* Check whether the head of the log points to an unmount record. In other
* words, determine whether the log is clean. If so, update the in-core state
@@ -1283,12 +1302,13 @@ xlog_check_unmount_rec(
} else {
hblks = 1;
}
- after_umount_blk = rhead_blk + hblks + BTOBB(be32_to_cpu(rhead->h_len));
- after_umount_blk = do_mod(after_umount_blk, log->l_logBBsize);
+
+ after_umount_blk = xlog_wrap_logbno(log,
+ rhead_blk + hblks + BTOBB(be32_to_cpu(rhead->h_len)));
+
if (*head_blk == after_umount_blk &&
be32_to_cpu(rhead->h_num_logops) == 1) {
- umount_data_blk = rhead_blk + hblks;
- umount_data_blk = do_mod(umount_data_blk, log->l_logBBsize);
+ umount_data_blk = xlog_wrap_logbno(log, rhead_blk + hblks);
error = xlog_bread(log, umount_data_blk, 1, bp, &offset);
if (error)
return error;
@@ -5459,9 +5479,7 @@ xlog_do_recovery_pass(
*/
if (blk_no + bblks <= log->l_logBBsize ||
blk_no >= log->l_logBBsize) {
- /* mod blk_no in case the header wrapped and
- * pushed it beyond the end of the log */
- rblk_no = do_mod(blk_no, log->l_logBBsize);
+ rblk_no = xlog_wrap_logbno(log, blk_no);
error = xlog_bread(log, rblk_no, bblks, dbp,
&offset);
if (error)
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 80bbfe604ce0..329d4d26c13e 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -301,8 +301,12 @@ xfs_rtallocate_extent_block(
/*
* If size should be a multiple of prod, make that so.
*/
- if (prod > 1 && (p = do_mod(bestlen, prod)))
- bestlen -= p;
+ if (prod > 1) {
+ div_u64_rem(bestlen, prod, &p);
+ if (p)
+ bestlen -= p;
+ }
+
/*
* Allocate besti for bestlen & return that.
*/
@@ -1263,7 +1267,7 @@ xfs_rtpick_extent(
b = (mp->m_sb.sb_rextents * ((resid << 1) + 1ULL)) >>
(log2 + 1);
if (b >= mp->m_sb.sb_rextents)
- b = do_mod(b, mp->m_sb.sb_rextents);
+ div64_u64_rem(b, mp->m_sb.sb_rextents, &b);
if (b + len > mp->m_sb.sb_rextents)
b = mp->m_sb.sb_rextents - len;
}
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3 V2] xfs: replace do_mod with native operations
2018-06-08 0:43 ` [PATCH 2/3 V2] " Dave Chinner
@ 2018-06-08 6:19 ` Christoph Hellwig
2018-06-08 7:31 ` Dave Chinner
0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2018-06-08 6:19 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Fri, Jun 08, 2018 at 10:43:57AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> do_mod() is a hold-over from when we have different sizes for file
> offsets and and other internal values for 40 bit XFS filesystems.
> Hence depending on build flags variables passed to do_mod() could
> change size. We no longer support those small format filesystems and
> hence everything is of fixed size theses days, even on 32 bit
> platforms.
>
> As such, we can convert all the do_mod() callers to platform
> optimised modulus operations as defined by linux/math64.h.
> Individual conversions depend on the types of variables being used.
I have to admit the XFS helpers are much more intuitive.
> 7 files changed, 66 insertions(+), 50 deletions(-)
And the diffstat agrees with me.
I'd rather see the XFS helpers lifted to the kernel on an as-needed
basis than making the code bigger and more hairy.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] xfs: clean up MIN/MAX
2018-06-07 5:27 ` [PATCH 3/3] xfs: clean up MIN/MAX Dave Chinner
2018-06-07 11:42 ` Brian Foster
@ 2018-06-08 6:23 ` Christoph Hellwig
1 sibling, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2018-06-08 6:23 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Thu, Jun 07, 2018 at 03:27:51PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Get rid of the MIN/MAX macros and just use the native min/max macros
> directly in the XFS code.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] xfs: move various type verifiers to common file
2018-06-07 5:27 ` [PATCH 1/3] xfs: move various type verifiers to common file Dave Chinner
2018-06-07 11:41 ` Brian Foster
2018-06-07 15:23 ` Darrick J. Wong
@ 2018-06-08 6:24 ` Christoph Hellwig
2 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2018-06-08 6:24 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Thu, Jun 07, 2018 at 03:27:49PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> New verification functions like xfs_verify_fsbno() and
> xfs_verify_agino() are spread across multiple files and different
> header files. They really don't fit cleanly into the places they've
> been put, and have wider scope than the current header includes.
>
> Move the type verifiers to a new file in libxfs (xfs-types.c) and
> the prototypes to xfs_types.h where they will be visible to all the
> code that uses the types.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/Makefile | 1 +
> fs/xfs/libxfs/xfs_alloc.c | 49 ----------
> fs/xfs/libxfs/xfs_alloc.h | 4 -
> fs/xfs/libxfs/xfs_ialloc.c | 90 ------------------
> fs/xfs/libxfs/xfs_ialloc.h | 7 --
> fs/xfs/libxfs/xfs_rtbitmap.c | 12 ---
> fs/xfs/libxfs/xfs_types.c | 173 +++++++++++++++++++++++++++++++++++
> fs/xfs/libxfs/xfs_types.h | 19 ++++
> fs/xfs/scrub/agheader.c | 2 +-
> 9 files changed, 194 insertions(+), 163 deletions(-)
> create mode 100644 fs/xfs/libxfs/xfs_types.c
>
> diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
> index 19a1f8064d8a..5333f7cbc95a 100644
> --- a/fs/xfs/Makefile
> +++ b/fs/xfs/Makefile
> @@ -50,6 +50,7 @@ xfs-y += $(addprefix libxfs/, \
> xfs_sb.o \
> xfs_symlink_remote.o \
> xfs_trans_resv.o \
> + xfs_types.o \
> )
> # xfs_rtbitmap is shared with libxfs
> xfs-$(CONFIG_XFS_RT) += $(addprefix libxfs/, \
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 1db50cfc0212..eef466260d43 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -3123,55 +3123,6 @@ xfs_alloc_query_all(
> return xfs_btree_query_all(cur, xfs_alloc_query_range_helper, &query);
> }
>
> -/* Find the size of the AG, in blocks. */
> -xfs_agblock_t
> -xfs_ag_block_count(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno)
> -{
> - ASSERT(agno < mp->m_sb.sb_agcount);
> -
> - if (agno < mp->m_sb.sb_agcount - 1)
> - return mp->m_sb.sb_agblocks;
> - return mp->m_sb.sb_dblocks - (agno * mp->m_sb.sb_agblocks);
> -}
> -
> -/*
> - * Verify that an AG block number pointer neither points outside the AG
> - * nor points at static metadata.
> - */
> -bool
> -xfs_verify_agbno(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno,
> - xfs_agblock_t agbno)
> -{
> - xfs_agblock_t eoag;
> -
> - eoag = xfs_ag_block_count(mp, agno);
> - if (agbno >= eoag)
> - return false;
> - if (agbno <= XFS_AGFL_BLOCK(mp))
> - return false;
> - return true;
> -}
> -
> -/*
> - * Verify that an FS block number pointer neither points outside the
> - * filesystem nor points at static AG metadata.
> - */
> -bool
> -xfs_verify_fsbno(
> - struct xfs_mount *mp,
> - xfs_fsblock_t fsbno)
> -{
> - xfs_agnumber_t agno = XFS_FSB_TO_AGNO(mp, fsbno);
> -
> - if (agno >= mp->m_sb.sb_agcount)
> - return false;
> - return xfs_verify_agbno(mp, agno, XFS_FSB_TO_AGBNO(mp, fsbno));
> -}
> -
> /* Is there a record covering a given extent? */
> int
> xfs_alloc_has_record(
> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
> index 1651d924aaf1..e716c993ac4c 100644
> --- a/fs/xfs/libxfs/xfs_alloc.h
> +++ b/fs/xfs/libxfs/xfs_alloc.h
> @@ -242,10 +242,6 @@ int xfs_alloc_query_range(struct xfs_btree_cur *cur,
> xfs_alloc_query_range_fn fn, void *priv);
> int xfs_alloc_query_all(struct xfs_btree_cur *cur, xfs_alloc_query_range_fn fn,
> void *priv);
> -xfs_agblock_t xfs_ag_block_count(struct xfs_mount *mp, xfs_agnumber_t agno);
> -bool xfs_verify_agbno(struct xfs_mount *mp, xfs_agnumber_t agno,
> - xfs_agblock_t agbno);
> -bool xfs_verify_fsbno(struct xfs_mount *mp, xfs_fsblock_t fsbno);
>
> int xfs_alloc_has_record(struct xfs_btree_cur *cur, xfs_agblock_t bno,
> xfs_extlen_t len, bool *exist);
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 3f551eb29157..8ec39dad62d7 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -2674,96 +2674,6 @@ xfs_ialloc_pagi_init(
> return 0;
> }
>
> -/* Calculate the first and last possible inode number in an AG. */
> -void
> -xfs_ialloc_agino_range(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno,
> - xfs_agino_t *first,
> - xfs_agino_t *last)
> -{
> - xfs_agblock_t bno;
> - xfs_agblock_t eoag;
> -
> - eoag = xfs_ag_block_count(mp, agno);
> -
> - /*
> - * Calculate the first inode, which will be in the first
> - * cluster-aligned block after the AGFL.
> - */
> - bno = round_up(XFS_AGFL_BLOCK(mp) + 1,
> - xfs_ialloc_cluster_alignment(mp));
> - *first = XFS_OFFBNO_TO_AGINO(mp, bno, 0);
> -
> - /*
> - * Calculate the last inode, which will be at the end of the
> - * last (aligned) cluster that can be allocated in the AG.
> - */
> - bno = round_down(eoag, xfs_ialloc_cluster_alignment(mp));
> - *last = XFS_OFFBNO_TO_AGINO(mp, bno, 0) - 1;
> -}
> -
> -/*
> - * Verify that an AG inode number pointer neither points outside the AG
> - * nor points at static metadata.
> - */
> -bool
> -xfs_verify_agino(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno,
> - xfs_agino_t agino)
> -{
> - xfs_agino_t first;
> - xfs_agino_t last;
> -
> - xfs_ialloc_agino_range(mp, agno, &first, &last);
> - return agino >= first && agino <= last;
> -}
> -
> -/*
> - * Verify that an FS inode number pointer neither points outside the
> - * filesystem nor points at static AG metadata.
> - */
> -bool
> -xfs_verify_ino(
> - struct xfs_mount *mp,
> - xfs_ino_t ino)
> -{
> - xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, ino);
> - xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ino);
> -
> - if (agno >= mp->m_sb.sb_agcount)
> - return false;
> - if (XFS_AGINO_TO_INO(mp, agno, agino) != ino)
> - return false;
> - return xfs_verify_agino(mp, agno, agino);
> -}
> -
> -/* Is this an internal inode number? */
> -bool
> -xfs_internal_inum(
> - struct xfs_mount *mp,
> - xfs_ino_t ino)
> -{
> - return ino == mp->m_sb.sb_rbmino || ino == mp->m_sb.sb_rsumino ||
> - (xfs_sb_version_hasquota(&mp->m_sb) &&
> - xfs_is_quota_inode(&mp->m_sb, ino));
> -}
> -
> -/*
> - * Verify that a directory entry's inode number doesn't point at an internal
> - * inode, empty space, or static AG metadata.
> - */
> -bool
> -xfs_verify_dir_ino(
> - struct xfs_mount *mp,
> - xfs_ino_t ino)
> -{
> - if (xfs_internal_inum(mp, ino))
> - return false;
> - return xfs_verify_ino(mp, ino);
> -}
> -
> /* Is there an inode record covering a given range of inode numbers? */
> int
> xfs_ialloc_has_inode_record(
> diff --git a/fs/xfs/libxfs/xfs_ialloc.h b/fs/xfs/libxfs/xfs_ialloc.h
> index 144f3eac9b93..90b09c5f163b 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.h
> +++ b/fs/xfs/libxfs/xfs_ialloc.h
> @@ -169,12 +169,5 @@ int xfs_inobt_insert_rec(struct xfs_btree_cur *cur, uint16_t holemask,
> int *stat);
>
> int xfs_ialloc_cluster_alignment(struct xfs_mount *mp);
> -void xfs_ialloc_agino_range(struct xfs_mount *mp, xfs_agnumber_t agno,
> - xfs_agino_t *first, xfs_agino_t *last);
> -bool xfs_verify_agino(struct xfs_mount *mp, xfs_agnumber_t agno,
> - xfs_agino_t agino);
> -bool xfs_verify_ino(struct xfs_mount *mp, xfs_ino_t ino);
> -bool xfs_internal_inum(struct xfs_mount *mp, xfs_ino_t ino);
> -bool xfs_verify_dir_ino(struct xfs_mount *mp, xfs_ino_t ino);
>
> #endif /* __XFS_IALLOC_H__ */
> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
> index ffc72075a44e..65fc4ed2e9a1 100644
> --- a/fs/xfs/libxfs/xfs_rtbitmap.c
> +++ b/fs/xfs/libxfs/xfs_rtbitmap.c
> @@ -1080,18 +1080,6 @@ xfs_rtalloc_query_all(
> return xfs_rtalloc_query_range(tp, &keys[0], &keys[1], fn, priv);
> }
>
> -/*
> - * Verify that an realtime block number pointer doesn't point off the
> - * end of the realtime device.
> - */
> -bool
> -xfs_verify_rtbno(
> - struct xfs_mount *mp,
> - xfs_rtblock_t rtbno)
> -{
> - return rtbno < mp->m_sb.sb_rblocks;
> -}
> -
> /* Is the given extent all free? */
> int
> xfs_rtalloc_extent_is_free(
> diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
> new file mode 100644
> index 000000000000..2e2a243cef2e
> --- /dev/null
> +++ b/fs/xfs/libxfs/xfs_types.c
> @@ -0,0 +1,173 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2000-2002,2005 Silicon Graphics, Inc.
> + * Copyright (C) 2017 Oracle.
> + * All Rights Reserved.
The all rights reserved statement doesn't relaly make sense with
the SPDX header. I guess this is copies from the exiasting code, but
we'll need to eventually sort it out.
Otherwise looks fine:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3 V2] xfs: replace do_mod with native operations
2018-06-08 6:19 ` Christoph Hellwig
@ 2018-06-08 7:31 ` Dave Chinner
0 siblings, 0 replies; 17+ messages in thread
From: Dave Chinner @ 2018-06-08 7:31 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Jun 07, 2018 at 11:19:09PM -0700, Christoph Hellwig wrote:
> On Fri, Jun 08, 2018 at 10:43:57AM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > do_mod() is a hold-over from when we have different sizes for file
> > offsets and and other internal values for 40 bit XFS filesystems.
> > Hence depending on build flags variables passed to do_mod() could
> > change size. We no longer support those small format filesystems and
> > hence everything is of fixed size theses days, even on 32 bit
> > platforms.
> >
> > As such, we can convert all the do_mod() callers to platform
> > optimised modulus operations as defined by linux/math64.h.
> > Individual conversions depend on the types of variables being used.
>
> I have to admit the XFS helpers are much more intuitive.
I've always found them opaque and clunky, especially with the return
value casts they seem to be frequently (but not consistently)
associated with.
> > 7 files changed, 66 insertions(+), 50 deletions(-)
>
> And the diffstat agrees with me.
No, it's doesn't, actually. The diffstat changes because I separated
calculations from logic operations, just like we do catching and
checking error returns from functions. i.e. the code goes from
this horrible mess or assignments, casts and calculations
intermingled with flow/logic decisions:
if ((args.mod = (xfs_extlen_t)do_mod(ap->offset, args.prod)))
args.mod = (xfs_extlen_t)(args.prod - args.mod);
Into a clear separation of calculation and logic statements like this:
div_u64_rem(ap->offset, args.prod, &args.mod);
if (args.mod)
args.mod = args.prod - args.mod;
That is also our preferred style for all the code (e.g. for catching
and checking errors). Yes, that means a slight growth in code size,
but I'll take that any day of the week if it means the code is
easier to read....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-06-08 7:31 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-07 5:27 [PATCH 0/3] xfs: minor cleanups Dave Chinner
2018-06-07 5:27 ` [PATCH 1/3] xfs: move various type verifiers to common file Dave Chinner
2018-06-07 11:41 ` Brian Foster
2018-06-07 15:23 ` Darrick J. Wong
2018-06-08 6:24 ` Christoph Hellwig
2018-06-07 5:27 ` [PATCH 2/3] xfs: replace do_mod with native operations Dave Chinner
2018-06-07 11:42 ` Brian Foster
2018-06-07 16:01 ` Darrick J. Wong
2018-06-07 22:23 ` Dave Chinner
2018-06-07 15:54 ` Darrick J. Wong
2018-06-07 22:28 ` Dave Chinner
2018-06-08 0:43 ` [PATCH 2/3 V2] " Dave Chinner
2018-06-08 6:19 ` Christoph Hellwig
2018-06-08 7:31 ` Dave Chinner
2018-06-07 5:27 ` [PATCH 3/3] xfs: clean up MIN/MAX Dave Chinner
2018-06-07 11:42 ` Brian Foster
2018-06-08 6:23 ` Christoph Hellwig
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.