Re: [xfs-masters] Re: [PATCH] fs/xfs: Handcrafted MIN/MAX macroremoval

From: Russell Cattelan
Date: Wed Oct 18 2006 - 13:53:48 EST

This change doesn't seem to do anything?

The existing macros already handle the types correctly,
are named such that it's obvious as to their purpose, so
it doesn't seem to make the code clearer.

This is also putting linux defined macros into the common
code and thereby creating more frustrations for the other XFS ports.


On Wed, 2006-10-18 at 16:27 +0530, Amol Lad wrote:
> Incroporated comments from Christoph Hellwig
>
> Cleanups done to use min/max macros from kernel.h. Handcrafted MIN/MAX
> macros are changed to use macros in kernel.h
>
> Tested using allmodconfig
>
> Signed-off-by: Amol Lad <amol@xxxxxxxxxxxxxxxxxxx>
> ---
> Christoph,
>
> I'm going on vacation. If you have more comments, I'll take them after I
> come back after 5 days..
>
> Thanks
> ---
> xfs_alloc.c | 24 +++++++++++-------------
> xfs_bmap.c | 32 ++++++++++++++------------------
> xfs_btree.h | 29 -----------------------------
> xfs_inode.c | 2 +-
> xfs_iomap.c | 2 +-
> xfs_rtalloc.c | 16 ++++++++--------
> xfs_rtalloc.h | 3 ---
> 7 files changed, 35 insertions(+), 73 deletions(-)
> ---
> diff -uprN -X linux-2.6.19-rc2-orig/Documentation/dontdiff linux-2.6.19-rc2-orig/fs/xfs/xfs_alloc.c linux-2.6.19-rc2/fs/xfs/xfs_alloc.c
> --- linux-2.6.19-rc2-orig/fs/xfs/xfs_alloc.c 2006-10-18 09:29:18.000000000 +0530
> +++ linux-2.6.19-rc2/fs/xfs/xfs_alloc.c 2006-10-18 16:13:44.000000000 +0530
> @@ -151,11 +151,11 @@ xfs_alloc_compute_diff(
> if (newbno1 >= freeend)
> newbno1 = NULLAGBLOCK;
> else
> - newlen1 = XFS_EXTLEN_MIN(wantlen, freeend - newbno1);
> + newlen1 = min_t(xfs_extlen_t, wantlen, freeend - newbno1);
> if (newbno2 < freebno)
> newbno2 = NULLAGBLOCK;
> else
> - newlen2 = XFS_EXTLEN_MIN(wantlen, freeend - newbno2);
> + newlen2 = min_t(xfs_extlen_t, wantlen, freeend - newbno2);
> if (newbno1 != NULLAGBLOCK && newbno2 != NULLAGBLOCK) {
> if (newlen1 < newlen2 ||
> (newlen1 == newlen2 &&
> @@ -686,7 +686,7 @@ xfs_alloc_ag_vextent_exact(
> * End of extent will be smaller of the freespace end and the
> * maximal requested end.
> */
> - end = XFS_AGBLOCK_MIN(fend, maxend);
> + end = min_t(xfs_agblock_t, fend, maxend);
> /*
> * Fix the length according to mod and prod if given.
> */
> @@ -850,7 +850,7 @@ xfs_alloc_ag_vextent_near(
> args->alignment, args->minlen,
> &ltbnoa, &ltlena))
> continue;
> - args->len = XFS_EXTLEN_MIN(ltlena, args->maxlen);
> + args->len = min_t(xfs_extlen_t, ltlena, args->maxlen);
> xfs_alloc_fix_len(args);
> ASSERT(args->len >= args->minlen);
> if (args->len < blen)
> @@ -1007,7 +1007,7 @@ xfs_alloc_ag_vextent_near(
> /*
> * Fix up the length.
> */
> - args->len = XFS_EXTLEN_MIN(ltlena, args->maxlen);
> + args->len = min_t(xfs_extlen_t, ltlena, args->maxlen);
> xfs_alloc_fix_len(args);
> rlen = args->len;
> ltdiff = xfs_alloc_compute_diff(args->agbno, rlen,
> @@ -1045,8 +1045,7 @@ xfs_alloc_ag_vextent_near(
> */
> if (gtlena >= args->minlen) {
> args->len =
> - XFS_EXTLEN_MIN(gtlena,
> - args->maxlen);
> + min_t(xfs_extlen_t, gtlena, args->maxlen);
> xfs_alloc_fix_len(args);
> rlen = args->len;
> gtdiff = xfs_alloc_compute_diff(
> @@ -1104,7 +1103,7 @@ xfs_alloc_ag_vextent_near(
> /*
> * Fix up the length.
> */
> - args->len = XFS_EXTLEN_MIN(gtlena, args->maxlen);
> + args->len = min_t(xfs_extlen_t, gtlena, args->maxlen);
> xfs_alloc_fix_len(args);
> rlen = args->len;
> gtdiff = xfs_alloc_compute_diff(args->agbno, rlen,
> @@ -1141,8 +1140,7 @@ xfs_alloc_ag_vextent_near(
> * compare the two and pick the best.
> */
> if (ltlena >= args->minlen) {
> - args->len = XFS_EXTLEN_MIN(
> - ltlena, args->maxlen);
> + args->len = min_t(xfs_extlen_t, ltlena, args->maxlen);
> xfs_alloc_fix_len(args);
> rlen = args->len;
> ltdiff = xfs_alloc_compute_diff(
> @@ -1221,7 +1219,7 @@ xfs_alloc_ag_vextent_near(
> * Fix up the length and compute the useful address.
> */
> ltend = ltbno + ltlen;
> - args->len = XFS_EXTLEN_MIN(ltlena, args->maxlen);
> + args->len = min_t(xfs_extlen_t, ltlena, args->maxlen);
> xfs_alloc_fix_len(args);
> if (!xfs_alloc_fix_minleft(args)) {
> TRACE_ALLOC("nominleft", args);
> @@ -1320,7 +1318,7 @@ xfs_alloc_ag_vextent_size(
> */
> xfs_alloc_compute_aligned(fbno, flen, args->alignment, args->minlen,
> &rbno, &rlen);
> - rlen = XFS_EXTLEN_MIN(args->maxlen, rlen);
> + rlen = min_t(xfs_extlen_t, args->maxlen, rlen);
> XFS_WANT_CORRUPTED_GOTO(rlen == 0 ||
> (rlen <= flen && rbno + rlen <= fbno + flen), error0);
> if (rlen < args->maxlen) {
> @@ -1346,7 +1344,7 @@ xfs_alloc_ag_vextent_size(
> break;
> xfs_alloc_compute_aligned(fbno, flen, args->alignment,
> args->minlen, &rbno, &rlen);
> - rlen = XFS_EXTLEN_MIN(args->maxlen, rlen);
> + rlen = min_t(xfs_extlen_t, args->maxlen, rlen);
> XFS_WANT_CORRUPTED_GOTO(rlen == 0 ||
> (rlen <= flen && rbno + rlen <= fbno + flen),
> error0);
> diff -uprN -X linux-2.6.19-rc2-orig/Documentation/dontdiff linux-2.6.19-rc2-orig/fs/xfs/xfs_bmap.c linux-2.6.19-rc2/fs/xfs/xfs_bmap.c
> --- linux-2.6.19-rc2-orig/fs/xfs/xfs_bmap.c 2006-10-18 09:29:18.000000000 +0530
> +++ linux-2.6.19-rc2/fs/xfs/xfs_bmap.c 2006-10-18 16:17:59.000000000 +0530
> @@ -1005,7 +1005,7 @@ xfs_bmap_add_extent_delay_real(
> LEFT.br_state)))
> goto done;
> }
> - temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
> + temp = min_t(xfs_filblks_t, xfs_bmap_worst_indlen(ip, temp),
> STARTBLOCKVAL(PREV.br_startblock));
> xfs_bmbt_set_startblock(ep, NULLSTARTBLOCK((int)temp));
> xfs_bmap_trace_post_update(fname, "LF|LC", ip, idx,
> @@ -1054,7 +1054,7 @@ xfs_bmap_add_extent_delay_real(
> if (error)
> goto done;
> }
> - temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
> + temp = min_t(xfs_filblks_t, xfs_bmap_worst_indlen(ip, temp),
> STARTBLOCKVAL(PREV.br_startblock) -
> (cur ? cur->bc_private.b.allocated : 0));
> ep = xfs_iext_get_ext(ifp, idx + 1);
> @@ -1101,7 +1101,7 @@ xfs_bmap_add_extent_delay_real(
> RIGHT.br_state)))
> goto done;
> }
> - temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
> + temp = min_t(xfs_filblks_t, xfs_bmap_worst_indlen(ip, temp),
> STARTBLOCKVAL(PREV.br_startblock));
> xfs_bmbt_set_startblock(ep, NULLSTARTBLOCK((int)temp));
> xfs_bmap_trace_post_update(fname, "RF|RC", ip, idx,
> @@ -1149,7 +1149,7 @@ xfs_bmap_add_extent_delay_real(
> if (error)
> goto done;
> }
> - temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
> + temp = min_t(xfs_filblks_t, xfs_bmap_worst_indlen(ip, temp),
> STARTBLOCKVAL(PREV.br_startblock) -
> (cur ? cur->bc_private.b.allocated : 0));
> ep = xfs_iext_get_ext(ifp, idx);
> @@ -3186,8 +3186,7 @@ xfs_bmap_del_extent(
> xfs_bmbt_set_blockcount(ep, temp);
> ifp->if_lastex = idx;
> if (delay) {
> - temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
> - da_old);
> + temp = min_t(xfs_filblks_t, xfs_bmap_worst_indlen(ip, temp), da_old);
> xfs_bmbt_set_startblock(ep, NULLSTARTBLOCK((int)temp));
> xfs_bmap_trace_post_update(fname, "2", ip, idx,
> whichfork);
> @@ -3215,8 +3214,7 @@ xfs_bmap_del_extent(
> xfs_bmbt_set_blockcount(ep, temp);
> ifp->if_lastex = idx;
> if (delay) {
> - temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
> - da_old);
> + temp = min_t(xfs_filblks_t, xfs_bmap_worst_indlen(ip, temp), da_old);
> xfs_bmbt_set_startblock(ep, NULLSTARTBLOCK((int)temp));
> xfs_bmap_trace_post_update(fname, "1", ip, idx,
> whichfork);
> @@ -4346,7 +4344,7 @@ xfs_bmap_first_unused(
> return 0;
> }
> lastaddr = off + xfs_bmbt_get_blockcount(ep);
> - max = XFS_FILEOFF_MAX(lastaddr, lowest);
> + max = max_t(xfs_fileoff_t, lastaddr, lowest);
> }
> *first_unused = max;
> return 0;
> @@ -4861,17 +4859,15 @@ xfs_bmapi(
> }
> } else if (wasdelay) {
> alen = (xfs_extlen_t)
> - XFS_FILBLKS_MIN(len,
> - (got.br_startoff +
> - got.br_blockcount) - bno);
> + min_t(xfs_filblks_t, len,
> + (got.br_startoff + got.br_blockcount) - bno);
> aoff = bno;
> } else {
> alen = (xfs_extlen_t)
> - XFS_FILBLKS_MIN(len, MAXEXTLEN);
> + min_t(xfs_filblks_t, len, MAXEXTLEN);
> if (!eof)
> alen = (xfs_extlen_t)
> - XFS_FILBLKS_MIN(alen,
> - got.br_startoff - bno);
> + min_t(xfs_filblks_t, alen, got.br_startoff - bno);
> aoff = bno;
> }
> minlen = (flags & XFS_BMAPI_CONTIG) ? alen : 1;
> @@ -5098,7 +5094,7 @@ xfs_bmapi(
> mval->br_startoff = bno;
> mval->br_startblock = HOLESTARTBLOCK;
> mval->br_blockcount =
> - XFS_FILBLKS_MIN(len, got.br_startoff - bno);
> + min_t(xfs_filblks_t, len, got.br_startoff - bno);
> mval->br_state = XFS_EXT_NORM;
> bno += mval->br_blockcount;
> len -= mval->br_blockcount;
> @@ -5133,7 +5129,7 @@ xfs_bmapi(
> * didn't overlap what was asked for.
> */
> mval->br_blockcount =
> - XFS_FILBLKS_MIN(end - bno, got.br_blockcount -
> + min_t(xfs_filblks_t, end - bno, got.br_blockcount -
> (bno - got.br_startoff));
> mval->br_state = got.br_state;
> ASSERT(mval->br_blockcount <= len);
> @@ -5473,7 +5469,7 @@ xfs_bunmapi(
> * Is the last block of this extent before the range
> * we're supposed to delete? If so, we're done.
> */
> - bno = XFS_FILEOFF_MIN(bno,
> + bno = min_t(xfs_fileoff_t, bno,
> got.br_startoff + got.br_blockcount - 1);
> if (bno < start)
> break;
> diff -uprN -X linux-2.6.19-rc2-orig/Documentation/dontdiff linux-2.6.19-rc2-orig/fs/xfs/xfs_btree.h linux-2.6.19-rc2/fs/xfs/xfs_btree.h
> --- linux-2.6.19-rc2-orig/fs/xfs/xfs_btree.h 2006-10-18 09:29:18.000000000 +0530
> +++ linux-2.6.19-rc2/fs/xfs/xfs_btree.h 2006-10-18 15:56:01.000000000 +0530
> @@ -440,35 +440,6 @@ xfs_btree_setbuf(
>
> #endif /* __KERNEL__ */
>
> -
> -/*
> - * Min and max functions for extlen, agblock, fileoff, and filblks types.
> - */
> -#define XFS_EXTLEN_MIN(a,b) \
> - ((xfs_extlen_t)(a) < (xfs_extlen_t)(b) ? \
> - (xfs_extlen_t)(a) : (xfs_extlen_t)(b))
> -#define XFS_EXTLEN_MAX(a,b) \
> - ((xfs_extlen_t)(a) > (xfs_extlen_t)(b) ? \
> - (xfs_extlen_t)(a) : (xfs_extlen_t)(b))
> -#define XFS_AGBLOCK_MIN(a,b) \
> - ((xfs_agblock_t)(a) < (xfs_agblock_t)(b) ? \
> - (xfs_agblock_t)(a) : (xfs_agblock_t)(b))
> -#define XFS_AGBLOCK_MAX(a,b) \
> - ((xfs_agblock_t)(a) > (xfs_agblock_t)(b) ? \
> - (xfs_agblock_t)(a) : (xfs_agblock_t)(b))
> -#define XFS_FILEOFF_MIN(a,b) \
> - ((xfs_fileoff_t)(a) < (xfs_fileoff_t)(b) ? \
> - (xfs_fileoff_t)(a) : (xfs_fileoff_t)(b))
> -#define XFS_FILEOFF_MAX(a,b) \
> - ((xfs_fileoff_t)(a) > (xfs_fileoff_t)(b) ? \
> - (xfs_fileoff_t)(a) : (xfs_fileoff_t)(b))
> -#define XFS_FILBLKS_MIN(a,b) \
> - ((xfs_filblks_t)(a) < (xfs_filblks_t)(b) ? \
> - (xfs_filblks_t)(a) : (xfs_filblks_t)(b))
> -#define XFS_FILBLKS_MAX(a,b) \
> - ((xfs_filblks_t)(a) > (xfs_filblks_t)(b) ? \
> - (xfs_filblks_t)(a) : (xfs_filblks_t)(b))
> -
> #define XFS_FSB_SANITY_CHECK(mp,fsb) \
> (XFS_FSB_TO_AGNO(mp, fsb) < mp->m_sb.sb_agcount && \
> XFS_FSB_TO_AGBNO(mp, fsb) < mp->m_sb.sb_agblocks)
> diff -uprN -X linux-2.6.19-rc2-orig/Documentation/dontdiff linux-2.6.19-rc2-orig/fs/xfs/xfs_inode.c linux-2.6.19-rc2/fs/xfs/xfs_inode.c
> --- linux-2.6.19-rc2-orig/fs/xfs/xfs_inode.c 2006-10-18 09:29:18.000000000 +0530
> +++ linux-2.6.19-rc2/fs/xfs/xfs_inode.c 2006-10-18 16:18:41.000000000 +0530
> @@ -1342,7 +1342,7 @@ xfs_file_last_byte(
> last_block = 0;
> }
> size_last_block = XFS_B_TO_FSB(mp, (xfs_ufsize_t)ip->i_d.di_size);
> - last_block = XFS_FILEOFF_MAX(last_block, size_last_block);
> + last_block = max_t(xfs_fileoff_t, last_block, size_last_block);
>
> last_byte = XFS_FSB_TO_B(mp, last_block);
> if (last_byte < 0) {
> diff -uprN -X linux-2.6.19-rc2-orig/Documentation/dontdiff linux-2.6.19-rc2-orig/fs/xfs/xfs_iomap.c linux-2.6.19-rc2/fs/xfs/xfs_iomap.c
> --- linux-2.6.19-rc2-orig/fs/xfs/xfs_iomap.c 2006-10-18 09:29:18.000000000 +0530
> +++ linux-2.6.19-rc2/fs/xfs/xfs_iomap.c 2006-10-18 15:54:13.000000000 +0530
> @@ -822,7 +822,7 @@ xfs_iomap_write_allocate(
> end_fsb = XFS_B_TO_FSB(mp, ip->i_d.di_size);
> xfs_bmap_last_offset(NULL, ip, &last_block,
> XFS_DATA_FORK);
> - last_block = XFS_FILEOFF_MAX(last_block, end_fsb);
> + last_block = max_t(xfs_fileoff_t,last_block, end_fsb);
> if ((map_start_fsb + count_fsb) > last_block) {
> count_fsb = last_block - map_start_fsb;
> if (count_fsb == 0) {
> diff -uprN -X linux-2.6.19-rc2-orig/Documentation/dontdiff linux-2.6.19-rc2-orig/fs/xfs/xfs_rtalloc.c linux-2.6.19-rc2/fs/xfs/xfs_rtalloc.c
> --- linux-2.6.19-rc2-orig/fs/xfs/xfs_rtalloc.c 2006-10-18 09:29:18.000000000 +0530
> +++ linux-2.6.19-rc2/fs/xfs/xfs_rtalloc.c 2006-10-18 16:06:37.000000000 +0530
> @@ -699,8 +699,8 @@ xfs_rtallocate_extent_size(
> * this summary level.
> */
> error = xfs_rtallocate_extent_block(mp, tp, i,
> - XFS_RTMAX(minlen, 1 << l),
> - XFS_RTMIN(maxlen, (1 << (l + 1)) - 1),
> + max_t(xfs_extlen_t, minlen, 1 << l),
> + min_t(xfs_extlen_t, maxlen, (1 << (l + 1)) - 1),
> len, &n, rbpp, rsb, prod, &r);
> if (error) {
> return error;
> @@ -1020,7 +1020,7 @@ xfs_rtcheck_range(
> /*
> * Compute first bit not examined.
> */
> - lastbit = XFS_RTMIN(bit + len, XFS_NBWORD);
> + lastbit = min_t(xfs_extlen_t, bit + len, XFS_NBWORD);
> /*
> * Mask of relevant bits.
> */
> @@ -1238,7 +1238,7 @@ xfs_rtfind_back(
> * Calculate first (leftmost) bit number to look at,
> * and mask for all the relevant bits in this word.
> */
> - firstbit = XFS_RTMAX((xfs_srtblock_t)(bit - len + 1), 0);
> + firstbit = max_t(xfs_srtblock_t, bit - len + 1, 0);
> mask = (((xfs_rtword_t)1 << (bit - firstbit + 1)) - 1) <<
> firstbit;
> /*
> @@ -1413,7 +1413,7 @@ xfs_rtfind_forw(
> * Calculate last (rightmost) bit number to look at,
> * and mask for all the relevant bits in this word.
> */
> - lastbit = XFS_RTMIN(bit + len, XFS_NBWORD);
> + lastbit = min_t(xfs_rtblock_t, bit + len, XFS_NBWORD);
> mask = (((xfs_rtword_t)1 << (lastbit - bit)) - 1) << bit;
> /*
> * Calculate the difference between the value there
> @@ -1724,7 +1724,7 @@ xfs_rtmodify_range(
> /*
> * Compute first bit not changed and mask of relevant bits.
> */
> - lastbit = XFS_RTMIN(bit + len, XFS_NBWORD);
> + lastbit = min_t(xfs_extlen_t, bit + len, XFS_NBWORD);
> mask = (((xfs_rtword_t)1 << (lastbit - bit)) - 1) << bit;
> /*
> * Set/clear the active bits.
> @@ -1998,7 +1998,7 @@ xfs_growfs_rt(
> nsbp->sb_rextsize = in->extsize;
> nsbp->sb_rbmblocks = bmbno + 1;
> nsbp->sb_rblocks =
> - XFS_RTMIN(nrblocks,
> + min_t(xfs_drfsbno_t, nrblocks,
> nsbp->sb_rbmblocks * NBBY *
> nsbp->sb_blocksize * nsbp->sb_rextsize);
> nsbp->sb_rextents = nsbp->sb_rblocks;
> @@ -2424,7 +2424,7 @@ xfs_rtprint_summary(
> if (c) {
> if (!p) {
> cmn_err(CE_DEBUG, "%Ld-%Ld:", 1LL << l,
> - XFS_RTMIN((1LL << l) +
> + min((1LL << l) +
> ((1LL << l) - 1LL),
> mp->m_sb.sb_rextents));
> p = 1;
> diff -uprN -X linux-2.6.19-rc2-orig/Documentation/dontdiff linux-2.6.19-rc2-orig/fs/xfs/xfs_rtalloc.h linux-2.6.19-rc2/fs/xfs/xfs_rtalloc.h
> --- linux-2.6.19-rc2-orig/fs/xfs/xfs_rtalloc.h 2006-08-24 02:46:33.000000000 +0530
> +++ linux-2.6.19-rc2/fs/xfs/xfs_rtalloc.h 2006-10-18 15:46:53.000000000 +0530
> @@ -57,9 +57,6 @@ struct xfs_trans;
> #define XFS_BITTOWORD(mp,bi) \
> ((int)(((bi) >> XFS_NBWORDLOG) & XFS_BLOCKWMASK(mp)))
>
> -#define XFS_RTMIN(a,b) ((a) < (b) ? (a) : (b))
> -#define XFS_RTMAX(a,b) ((a) > (b) ? (a) : (b))
> -
> #define XFS_RTLOBIT(w) xfs_lowbit32(w)
> #define XFS_RTHIBIT(w) xfs_highbit32(w)
>
>

--
Russell Cattelan <cattelan@xxxxxxxxxxx>

Attachment: signature.asc
Description: This is a digitally signed message part