Re: [xfs-masters] Re: [PATCH] fs/xfs: remove duplicated defines

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



David Chinner wrote:
On Tue, Nov 13, 2007 at 04:35:40PM +1100, Timothy Shimmin wrote:
David Chinner wrote:
Perhaps we should look at cleaning up the cusers of offtoc, offtoct, etc
and killing BPCSHIFT altogether....

Yeah, I had a quick look before, but I will look closer again ;-)

 > egrep -Ir 'offtoc|ctoooff' . | egrep -v "anot|tag"
./linux-2.6/xfs_lrw.c:                                  ctooff(offtoct(*offset)),
./linux-2.6/xfs_lrw.c:                                  ctooff(offtoct(pos)), -1);
./linux-2.6/xfs_lrw.c:                                  ctooff(offtoct(pos)),
./linux-2.6/xfs_linux.h:#define offtoc(x)       (((__uint64_t)(x)+(NBPC-1))>>BPCSHIFT)
./linux-2.6/xfs_linux.h:#define offtoct(x)      ((xfs_off_t)(x)>>BPCSHIFT)
./xfs_vnodeops.c:                               ctooff(offtoct(ioffset)), -1);
./xfs_vnodeops.c:                               ctooff(offtoct(ioffset)),

So we basically just use:

ctooff(offtoct(pos))

where
#define	ctooff(x)	((xfs_off_t)(x)<<BPCSHIFT)
#define offtoct(x)      ((xfs_off_t)(x)>>BPCSHIFT)
#define BPCSHIFT        PAGE_SHIFT      /* LOG2(NBPC) if exact */

seems basically to be a:

#define round_down_page(x) ((x) & ~(PAGE_SIZE - 1))

or just use a
round_down(x, PAGE_SIZE)
and
define the round_down for size which is power of 2.

Like in asm-x86_64/proto.h
#define round_up(x,y) (((x) + (y) - 1) & ~((y)-1))
#define round_down(x,y) ((x) & ~((y)-1))

What way do you reckon?

Neither ;)

:)


Just replace them with (val & PAGE_CACHE_MASK)

Ah, there is already a macro for ~(PAGE_SIZE - 1) that would
be good. (You can tell I look at a lot of this page code:)

Actually, the code in xfs_vnodeops.c culd probably just be removed; the
ioffset variable is already rounded to a multiple of page size....

Yep...
        rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, NBPP);
        ioffset = offset & ~(rounding - 1);

Looks like we used to use ioffset directly in a call to xfs_inval_cached_pages()
and when hch moved to VOP_FLUSH_INVALPAGES we used the ctoff(offtoct(...

Cheers,

dave.

Remove the BPCSHIFT based macros from XFS.

The BPCSHIFT based macros, btoc*, ctob*, offtoc* and ctooff
are either not used or don't need to be used.
Initial patch and motivation from Nicolas Kaiser.

Signed-Off-By: Tim Shimmin <tes@xxxxxxx>
---
 b/fs/xfs/linux-2.6/xfs_linux.h |   21 ---------------------
 b/fs/xfs/linux-2.6/xfs_lrw.c   |    9 ++++-----
 b/fs/xfs/xfs_vnodeops.c        |    7 ++-----
 3 files changed, 6 insertions(+), 31 deletions(-)

===========================================================================
Index: fs/xfs/linux-2.6/xfs_linux.h
===========================================================================

--- a/fs/xfs/linux-2.6/xfs_linux.h	2007-11-14 13:02:46.000000000 +1100
+++ b/fs/xfs/linux-2.6/xfs_linux.h	2007-11-14 12:40:25.297746498 +1100
@@ -159,27 +159,6 @@
 /* number of BB's per block device block */
 #define	BLKDEV_BB		BTOBB(BLKDEV_IOSIZE)

-/* bytes to clicks */
-#define	btoc(x)		(((__psunsigned_t)(x)+(NBPC-1))>>BPCSHIFT)
-#define	btoct(x)	((__psunsigned_t)(x)>>BPCSHIFT)
-#define	btoc64(x)	(((__uint64_t)(x)+(NBPC-1))>>BPCSHIFT)
-#define	btoct64(x)	((__uint64_t)(x)>>BPCSHIFT)
-
-/* off_t bytes to clicks */
-#define offtoc(x)       (((__uint64_t)(x)+(NBPC-1))>>BPCSHIFT)
-#define offtoct(x)      ((xfs_off_t)(x)>>BPCSHIFT)
-
-/* clicks to off_t bytes */
-#define	ctooff(x)	((xfs_off_t)(x)<<BPCSHIFT)
-
-/* clicks to bytes */
-#define	ctob(x)		((__psunsigned_t)(x)<<BPCSHIFT)
-#define btoct(x)        ((__psunsigned_t)(x)>>BPCSHIFT)
-#define	ctob64(x)	((__uint64_t)(x)<<BPCSHIFT)
-
-/* bytes to clicks */
-#define btoc(x)         (((__psunsigned_t)(x)+(NBPC-1))>>BPCSHIFT)
-
 #define ENOATTR		ENODATA		/* Attribute not found */
 #define EWRONGFS	EINVAL		/* Mount with wrong filesystem type */
 #define EFSCORRUPTED	EUCLEAN		/* Filesystem is corrupted */

===========================================================================
Index: fs/xfs/linux-2.6/xfs_lrw.c
===========================================================================

--- a/fs/xfs/linux-2.6/xfs_lrw.c	2007-11-14 13:02:46.000000000 +1100
+++ b/fs/xfs/linux-2.6/xfs_lrw.c	2007-11-14 12:36:59.920080014 +1100
@@ -254,9 +254,8 @@ xfs_read(

 	if (unlikely(ioflags & IO_ISDIRECT)) {
 		if (VN_CACHED(vp))
-			ret = xfs_flushinval_pages(ip,
-					ctooff(offtoct(*offset)),
-					-1, FI_REMAPF_LOCKED);
+			ret = xfs_flushinval_pages(ip, (*offset & PAGE_MASK),
+						    -1, FI_REMAPF_LOCKED);
 		mutex_unlock(&inode->i_mutex);
 		if (ret) {
 			xfs_iunlock(ip, XFS_IOLOCK_SHARED);
@@ -742,9 +741,9 @@ retry:
 		if (VN_CACHED(vp)) {
 			WARN_ON(need_i_mutex == 0);
 			xfs_inval_cached_trace(xip, pos, -1,
-					ctooff(offtoct(pos)), -1);
+					(pos & PAGE_MASK), -1);
 			error = xfs_flushinval_pages(xip,
-					ctooff(offtoct(pos)),
+					(pos & PAGE_MASK),
 					-1, FI_REMAPF_LOCKED);
 			if (error)
 				goto out_unlock_internal;

===========================================================================
Index: fs/xfs/xfs_vnodeops.c
===========================================================================

--- a/fs/xfs/xfs_vnodeops.c	2007-11-14 13:02:46.000000000 +1100
+++ b/fs/xfs/xfs_vnodeops.c	2007-11-14 12:32:36.182055952 +1100
@@ -4168,11 +4168,8 @@ xfs_free_file_space(
 	ioffset = offset & ~(rounding - 1);

 	if (VN_CACHED(vp) != 0) {
-		xfs_inval_cached_trace(ip, ioffset, -1,
-				ctooff(offtoct(ioffset)), -1);
-		error = xfs_flushinval_pages(ip,
-				ctooff(offtoct(ioffset)),
-				-1, FI_REMAPF_LOCKED);
+		xfs_inval_cached_trace(ip, ioffset, -1, ioffset, -1);
+		error = xfs_flushinval_pages(ip, ioffset, -1, FI_REMAPF_LOCKED);
 		if (error)
 			goto out_unlock_iolock;
 	}

-
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux