2006-10-18 08:25:41

by Amol Lad

[permalink] [raw]
Subject: [PATCH] fs/xfs: Handcrafted MIN/MAX macro removal


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 <[email protected]>
---
xfs_btree.h | 34 ++++++++++------------------------
xfs_rtalloc.c | 20 ++++++++++----------
xfs_rtalloc.h | 3 ---
---
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 11:25:46.000000000 +0530
@@ -18,6 +18,8 @@
#ifndef __XFS_BTREE_H__
#define __XFS_BTREE_H__

+#include <linux/kernel.h>
+
struct xfs_buf;
struct xfs_bmap_free;
struct xfs_inode;
@@ -444,30 +446,14 @@ xfs_btree_setbuf(
/*
* 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_EXTLEN_MIN(a,b) (min_t(xfs_extlen_t,a,b))
+#define XFS_EXTLEN_MAX(a,b) (max_t(xfs_extlen_t,a,b))
+#define XFS_AGBLOCK_MIN(a,b) (min_t(xfs_agblock_t,a,b))
+#define XFS_AGBLOCK_MAX(a,b) (max_t(xfs_agblock_t,a,b))
+#define XFS_FILEOFF_MIN(a,b) (min_t(xfs_fileoff_t,a,b))
+#define XFS_FILEOFF_MAX(a,b) (max_t(xfs_fileoff_t,a,b))
+#define XFS_FILBLKS_MIN(a,b) (min_t(xfs_filblks_t,a,b))
+#define XFS_FILBLKS_MAX(a,b) (max_t(xfs_filblks_t,a,b))

#define XFS_FSB_SANITY_CHECK(mp,fsb) \
(XFS_FSB_TO_AGNO(mp, fsb) < mp->m_sb.sb_agcount && \
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 11:25:46.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(minlen, (xfs_extlen_t)(1 << l)),
+ min(maxlen, (xfs_extlen_t)((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(bit + len, (xfs_extlen_t)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((xfs_srtblock_t)(bit - len + 1), (xfs_srtblock_t)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(bit + len, (xfs_rtblock_t)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(bit + len, (xfs_extlen_t)XFS_NBWORD);
mask = (((xfs_rtword_t)1 << (lastbit - bit)) - 1) << bit;
/*
* Set/clear the active bits.
@@ -1998,9 +1998,9 @@ xfs_growfs_rt(
nsbp->sb_rextsize = in->extsize;
nsbp->sb_rbmblocks = bmbno + 1;
nsbp->sb_rblocks =
- XFS_RTMIN(nrblocks,
- nsbp->sb_rbmblocks * NBBY *
- nsbp->sb_blocksize * nsbp->sb_rextsize);
+ min(nrblocks,
+ (xfs_drfsbno_t)(nsbp->sb_rbmblocks * NBBY *
+ nsbp->sb_blocksize * nsbp->sb_rextsize));
nsbp->sb_rextents = nsbp->sb_rblocks;
do_div(nsbp->sb_rextents, nsbp->sb_rextsize);
nsbp->sb_rextslog = xfs_highbit32(nsbp->sb_rextents);
@@ -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 11:25:46.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)






2006-10-18 09:50:04

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [xfs-masters] [PATCH] fs/xfs: Handcrafted MIN/MAX macro removal

On Wed, Oct 18, 2006 at 01:58:52PM +0530, Amol Lad wrote:
> --- 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 11:25:46.000000000 +0530
> @@ -18,6 +18,8 @@
> #ifndef __XFS_BTREE_H__
> #define __XFS_BTREE_H__
>
> +#include <linux/kernel.h>
> +

This file is also used in libxfs, so you can't just include this header
directly. On the other hand it should always get kernel.h through
xfs_linux.h anyway so you might aswell just leave the include out.

> +#define XFS_EXTLEN_MIN(a,b) (min_t(xfs_extlen_t,a,b))
> +#define XFS_EXTLEN_MAX(a,b) (max_t(xfs_extlen_t,a,b))
> +#define XFS_AGBLOCK_MIN(a,b) (min_t(xfs_agblock_t,a,b))
> +#define XFS_AGBLOCK_MAX(a,b) (max_t(xfs_agblock_t,a,b))
> +#define XFS_FILEOFF_MIN(a,b) (min_t(xfs_fileoff_t,a,b))
> +#define XFS_FILEOFF_MAX(a,b) (max_t(xfs_fileoff_t,a,b))
> +#define XFS_FILBLKS_MIN(a,b) (min_t(xfs_filblks_t,a,b))
> +#define XFS_FILBLKS_MAX(a,b) (max_t(xfs_filblks_t,a,b))

At this point we might aswell kill these UGLY SHOUTING macros and
use min_t/max_t (or just min/max where appropinquate) directly.

> error = xfs_rtallocate_extent_block(mp, tp, i,
> - XFS_RTMAX(minlen, 1 << l),
> - XFS_RTMIN(maxlen, (1 << (l + 1)) - 1),
> + max(minlen, (xfs_extlen_t)(1 << l)),
> + min(maxlen, (xfs_extlen_t)((1 << (l + 1)) - 1)),

these would be a lot more readable as

max_t(xfs_extlen_t, minlen, (1 << l)),
min_t(xfs_extlen_t, maxlen,
((1 << (l + 1)) - 1)),

wouldn, it?

> - lastbit = XFS_RTMIN(bit + len, XFS_NBWORD);
> + lastbit = min(bit + len, (xfs_extlen_t)XFS_NBWORD);

> - firstbit = XFS_RTMAX((xfs_srtblock_t)(bit - len + 1), 0);
> + firstbit = max((xfs_srtblock_t)(bit - len + 1), (xfs_srtblock_t)0);

> - lastbit = XFS_RTMIN(bit + len, XFS_NBWORD);
> + lastbit = min(bit + len, (xfs_rtblock_t)XFS_NBWORD);

> - lastbit = XFS_RTMIN(bit + len, XFS_NBWORD);
> + lastbit = min(bit + len, (xfs_extlen_t)XFS_NBWORD);

> - XFS_RTMIN(nrblocks,
> - nsbp->sb_rbmblocks * NBBY *
> - nsbp->sb_blocksize * nsbp->sb_rextsize);
> + min(nrblocks,
> + (xfs_drfsbno_t)(nsbp->sb_rbmblocks * NBBY *
> + nsbp->sb_blocksize * nsbp->sb_rextsize));

ditto

2006-10-18 10:54:36

by Amol Lad

[permalink] [raw]
Subject: Re: [PATCH] fs/xfs: Handcrafted MIN/MAX macro removal

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 <[email protected]>
---
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)



2006-10-18 17:53:31

by Russell Cattelan

[permalink] [raw]
Subject: Re: [xfs-masters] Re: [PATCH] fs/xfs: Handcrafted MIN/MAX macro removal

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 <[email protected]>
> ---
> 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 <[email protected]>


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2006-10-18 20:07:51

by Pekka Enberg

[permalink] [raw]
Subject: Re: Re: [xfs-masters] Re: [PATCH] fs/xfs: Handcrafted MIN/MAX macro removal

On 10/18/06, Russell Cattelan <[email protected]> wrote:
> This change doesn't seem to do anything?

It kills useless wrappers from kernel code making it easier to read
and maintain for _Linux_ developers.

2006-10-19 01:52:45

by David Chinner

[permalink] [raw]
Subject: Re: [xfs-masters] [PATCH] fs/xfs: Handcrafted MIN/MAX macro removal

On Wed, Oct 18, 2006 at 11:49:56AM +0200, Christoph Hellwig wrote:
> On Wed, Oct 18, 2006 at 01:58:52PM +0530, Amol Lad wrote:
> > --- 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 11:25:46.000000000 +0530
> > @@ -18,6 +18,8 @@
> > #ifndef __XFS_BTREE_H__
> > #define __XFS_BTREE_H__
> >
> > +#include <linux/kernel.h>
> > +
>
> This file is also used in libxfs, so you can't just include this header
> directly. On the other hand it should always get kernel.h through
> xfs_linux.h anyway so you might aswell just leave the include out.

*nod* - the include is not needed here.

> > +#define XFS_EXTLEN_MIN(a,b) (min_t(xfs_extlen_t,a,b))
> > +#define XFS_EXTLEN_MAX(a,b) (max_t(xfs_extlen_t,a,b))
> > +#define XFS_AGBLOCK_MIN(a,b) (min_t(xfs_agblock_t,a,b))
> > +#define XFS_AGBLOCK_MAX(a,b) (max_t(xfs_agblock_t,a,b))
> > +#define XFS_FILEOFF_MIN(a,b) (min_t(xfs_fileoff_t,a,b))
> > +#define XFS_FILEOFF_MAX(a,b) (max_t(xfs_fileoff_t,a,b))
> > +#define XFS_FILBLKS_MIN(a,b) (min_t(xfs_filblks_t,a,b))
> > +#define XFS_FILBLKS_MAX(a,b) (max_t(xfs_filblks_t,a,b))
>
> At this point we might aswell kill these UGLY SHOUTING macros and
> use min_t/max_t (or just min/max where appropinquate) directly.

No way.

> > error = xfs_rtallocate_extent_block(mp, tp, i,
> > - XFS_RTMAX(minlen, 1 << l),
> > - XFS_RTMIN(maxlen, (1 << (l + 1)) - 1),
> > + max(minlen, (xfs_extlen_t)(1 << l)),
> > + min(maxlen, (xfs_extlen_t)((1 << (l + 1)) - 1)),
>
> these would be a lot more readable as
>
> max_t(xfs_extlen_t, minlen, (1 << l)),
> min_t(xfs_extlen_t, maxlen,
> ((1 << (l + 1)) - 1)),
>
> wouldn, it?

No. I look at that XFS_RTMAX() macro and I *know* we are doing the
correct compare for that type of object. I look at the max_t version
and think to myself "is that correct, or is it done differently
elsewhere?". That macro says "I'm doing the right thing", and that
is invaluable information when it comes to reading the code and
tracking down bugs.

By removing the macros completely, we no longer know exactly where
in the code we compare distinct types, nor do we have a consistent
manner in which to compare them. When it comes to doing future mods,
we're now completely reliant on the developer finding all the places
we compare the distinct types and/or adding new compares using the
same code. This is a Bad Thing.

So while I'm happy with the original patch that just changes the
macro implementation (minus the include), I really dislike the
followup patch that removes the macros altogether because of these
reasons.

Cheers,

Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group