[PATCH] fs/xfs: remove duplicated defines

[PATCH] fs/xfs: remove duplicated defines

[Nicolas Kaiser]

Remove duplicated defines.

Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
---

 fs/xfs/linux-2.6/xfs_linux.h |    4 ----
 1 file changed, 4 deletions(-)

--- a/fs/xfs/linux-2.6/xfs_linux.h	2007-11-07 11:26:20.000000000 +0100
+++ b/fs/xfs/linux-2.6/xfs_linux.h	2007-11-11 13:07:11.000000000 +0100
@@ -167,12 +167,8 @@
 
 /* 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 */

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

[Eric Sandeen]

Nicolas Kaiser wrote:
> Remove duplicated defines.
> 
> Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
> ---

Heh, each defined twice, but used 0 times in the kernel.  Could probably
just remove them altogether (though I guess btoc is used in
xfstests/ltp/doio.c in userspace/xfstests, but that's the *only* place)

-Eric

>  fs/xfs/linux-2.6/xfs_linux.h |    4 ----
>  1 file changed, 4 deletions(-)
> 
> --- a/fs/xfs/linux-2.6/xfs_linux.h	2007-11-07 11:26:20.000000000 +0100
> +++ b/fs/xfs/linux-2.6/xfs_linux.h	2007-11-11 13:07:11.000000000 +0100
> @@ -167,12 +167,8 @@
>  
>  /* 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 */
> 
> 

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

[Eric Sandeen]

Nicolas Kaiser wrote:
> Remove duplicated defines.
> 
> Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
> ---

Heh, each defined twice, but used 0 times in the kernel.  Could probably
just remove them altogether (though I guess btoc is used in
xfstests/ltp/doio.c in userspace/xfstests, but that's the *only* place)

-Eric

>  fs/xfs/linux-2.6/xfs_linux.h |    4 ----
>  1 file changed, 4 deletions(-)
> 
> --- a/fs/xfs/linux-2.6/xfs_linux.h	2007-11-07 11:26:20.000000000 +0100
> +++ b/fs/xfs/linux-2.6/xfs_linux.h	2007-11-11 13:07:11.000000000 +0100
> @@ -167,12 +167,8 @@
>  
>  /* 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 */
> 
> 

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

[Timothy Shimmin]

Eric Sandeen wrote:
> Nicolas Kaiser wrote:
>> Remove duplicated defines.
>>
>> Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
>> ---
> 
> Heh, each defined twice, but used 0 times in the kernel.  Could probably
> just remove them altogether (though I guess btoc is used in
> xfstests/ltp/doio.c in userspace/xfstests, but that's the *only* place)
> 
Yes, that's what I was just noticing - how they are not used anyway.

 > grep -Ir btoc . | egrep -v 'tag|anot'
./linux-2.6/xfs_linux.h:#define btoc(x)         (((__psunsigned_t)(x)+(NBPC-1))>>BPCSHIFT)
./linux-2.6/xfs_linux.h:#define btoct(x)        ((__psunsigned_t)(x)>>BPCSHIFT)
./linux-2.6/xfs_linux.h:#define btoc64(x)       (((__uint64_t)(x)+(NBPC-1))>>BPCSHIFT)
./linux-2.6/xfs_linux.h:#define btoct64(x)      ((__uint64_t)(x)>>BPCSHIFT)
./linux-2.6/xfs_linux.h:#define btoct(x)        ((__psunsigned_t)(x)>>BPCSHIFT)
./linux-2.6/xfs_linux.h:#define btoc(x)         (((__psunsigned_t)(x)+(NBPC-1))>>BPCSHIFT)
./linux-2.6/xfs_buf.c:  page_count = xfs_buf_btoc(end) - xfs_buf_btoct(bp->b_file_offset);
./linux-2.6/xfs_buf.c:          page = bp->b_pages[xfs_buf_btoct(boff + bp->b_offset)];
./linux-2.6/xfs_buf.h:#define xfs_buf_btoc(dd)  (((dd) + PAGE_CACHE_SIZE-1) >> PAGE_CACHE_SHIFT)
./linux-2.6/xfs_buf.h:#define xfs_buf_btoct(dd) ((dd) >> PAGE_CACHE_SHIFT)

--Tim

> -Eric
> 
>>  fs/xfs/linux-2.6/xfs_linux.h |    4 ----
>>  1 file changed, 4 deletions(-)
>>
>> --- a/fs/xfs/linux-2.6/xfs_linux.h	2007-11-07 11:26:20.000000000 +0100
>> +++ b/fs/xfs/linux-2.6/xfs_linux.h	2007-11-11 13:07:11.000000000 +0100
>> @@ -167,12 +167,8 @@
>>  
>>  /* 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 */
>>
>>
> 

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

[Timothy Shimmin]

Eric Sandeen wrote:
> Nicolas Kaiser wrote:
>> Remove duplicated defines.
>>
>> Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
>> ---
> 
> Heh, each defined twice, but used 0 times in the kernel.  Could probably
> just remove them altogether (though I guess btoc is used in
> xfstests/ltp/doio.c in userspace/xfstests, but that's the *only* place)
> 
Yes, that's what I was just noticing - how they are not used anyway.

 > grep -Ir btoc . | egrep -v 'tag|anot'
./linux-2.6/xfs_linux.h:#define btoc(x)         (((__psunsigned_t)(x)+(NBPC-1))>>BPCSHIFT)
./linux-2.6/xfs_linux.h:#define btoct(x)        ((__psunsigned_t)(x)>>BPCSHIFT)
./linux-2.6/xfs_linux.h:#define btoc64(x)       (((__uint64_t)(x)+(NBPC-1))>>BPCSHIFT)
./linux-2.6/xfs_linux.h:#define btoct64(x)      ((__uint64_t)(x)>>BPCSHIFT)
./linux-2.6/xfs_linux.h:#define btoct(x)        ((__psunsigned_t)(x)>>BPCSHIFT)
./linux-2.6/xfs_linux.h:#define btoc(x)         (((__psunsigned_t)(x)+(NBPC-1))>>BPCSHIFT)
./linux-2.6/xfs_buf.c:  page_count = xfs_buf_btoc(end) - xfs_buf_btoct(bp->b_file_offset);
./linux-2.6/xfs_buf.c:          page = bp->b_pages[xfs_buf_btoct(boff + bp->b_offset)];
./linux-2.6/xfs_buf.h:#define xfs_buf_btoc(dd)  (((dd) + PAGE_CACHE_SIZE-1) >> PAGE_CACHE_SHIFT)
./linux-2.6/xfs_buf.h:#define xfs_buf_btoct(dd) ((dd) >> PAGE_CACHE_SHIFT)

--Tim

> -Eric
> 
>>  fs/xfs/linux-2.6/xfs_linux.h |    4 ----
>>  1 file changed, 4 deletions(-)
>>
>> --- a/fs/xfs/linux-2.6/xfs_linux.h	2007-11-07 11:26:20.000000000 +0100
>> +++ b/fs/xfs/linux-2.6/xfs_linux.h	2007-11-11 13:07:11.000000000 +0100
>> @@ -167,12 +167,8 @@
>>  
>>  /* 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 */
>>
>>
> 

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

[Timothy Shimmin]

[-- Attachment #1: Type: text/plain, Size: 2358 bytes --]

Timothy Shimmin wrote:
> Eric Sandeen wrote:
>> Nicolas Kaiser wrote:
>>> Remove duplicated defines.
>>>
>>> Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
>>> ---
>>
>> Heh, each defined twice, but used 0 times in the kernel.  Could probably
>> just remove them altogether (though I guess btoc is used in
>> xfstests/ltp/doio.c in userspace/xfstests, but that's the *only* place)
>>
> Yes, that's what I was just noticing - how they are not used anyway.
> 
>  > grep -Ir btoc . | egrep -v 'tag|anot'
> ./linux-2.6/xfs_linux.h:#define btoc(x)         
> (((__psunsigned_t)(x)+(NBPC-1))>>BPCSHIFT)
> ./linux-2.6/xfs_linux.h:#define btoct(x)        
> ((__psunsigned_t)(x)>>BPCSHIFT)
> ./linux-2.6/xfs_linux.h:#define btoc64(x)       
> (((__uint64_t)(x)+(NBPC-1))>>BPCSHIFT)
> ./linux-2.6/xfs_linux.h:#define btoct64(x)      ((__uint64_t)(x)>>BPCSHIFT)
> ./linux-2.6/xfs_linux.h:#define btoct(x)        
> ((__psunsigned_t)(x)>>BPCSHIFT)
> ./linux-2.6/xfs_linux.h:#define btoc(x)         
> (((__psunsigned_t)(x)+(NBPC-1))>>BPCSHIFT)
> ./linux-2.6/xfs_buf.c:  page_count = xfs_buf_btoc(end) - 
> xfs_buf_btoct(bp->b_file_offset);
> ./linux-2.6/xfs_buf.c:          page = bp->b_pages[xfs_buf_btoct(boff + 
> bp->b_offset)];
> ./linux-2.6/xfs_buf.h:#define xfs_buf_btoc(dd)  (((dd) + 
> PAGE_CACHE_SIZE-1) >> PAGE_CACHE_SHIFT)
> ./linux-2.6/xfs_buf.h:#define xfs_buf_btoct(dd) ((dd) >> PAGE_CACHE_SHIFT)
> 
> --Tim
> 
>> -Eric
>>
>>>  fs/xfs/linux-2.6/xfs_linux.h |    4 ----
>>>  1 file changed, 4 deletions(-)
>>>
>>> --- a/fs/xfs/linux-2.6/xfs_linux.h    2007-11-07 11:26:20.000000000 
>>> +0100
>>> +++ b/fs/xfs/linux-2.6/xfs_linux.h    2007-11-11 13:07:11.000000000 
>>> +0100
>>> @@ -167,12 +167,8 @@
>>>  
>>>  /* 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 */
>>>
>>>
>>
> 

So I'm thinking of "seeing" your patch and raising you by my attached patch :-)

--Tim

[-- Attachment #2: btoc.patch --]
[-- Type: text/x-patch, Size: 1377 bytes --]


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

--- a/fs/xfs/linux-2.6/xfs_linux.h	2007-11-12 11:24:05.000000000 +1100
+++ b/fs/xfs/linux-2.6/xfs_linux.h	2007-11-12 11:14:22.818831666 +1100
@@ -159,12 +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)
@@ -172,14 +166,6 @@
 /* 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 */

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

[Timothy Shimmin]

Timothy Shimmin wrote:
> Timothy Shimmin wrote:
>> Eric Sandeen wrote:
>>> Nicolas Kaiser wrote:
>>>> Remove duplicated defines.
>>>>
>>>> Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
>>>> ---
>>> Heh, each defined twice, but used 0 times in the kernel.  Could probably
>>> just remove them altogether (though I guess btoc is used in
>>> xfstests/ltp/doio.c in userspace/xfstests, but that's the *only* place)
>>>
>> Yes, that's what I was just noticing - how they are not used anyway.
>>
>>  > grep -Ir btoc . | egrep -v 'tag|anot'
>> ./linux-2.6/xfs_linux.h:#define btoc(x)         
>> (((__psunsigned_t)(x)+(NBPC-1))>>BPCSHIFT)
>> ./linux-2.6/xfs_linux.h:#define btoct(x)        
>> ((__psunsigned_t)(x)>>BPCSHIFT)
>> ./linux-2.6/xfs_linux.h:#define btoc64(x)       
>> (((__uint64_t)(x)+(NBPC-1))>>BPCSHIFT)
>> ./linux-2.6/xfs_linux.h:#define btoct64(x)      ((__uint64_t)(x)>>BPCSHIFT)
>> ./linux-2.6/xfs_linux.h:#define btoct(x)        
>> ((__psunsigned_t)(x)>>BPCSHIFT)
>> ./linux-2.6/xfs_linux.h:#define btoc(x)         
>> (((__psunsigned_t)(x)+(NBPC-1))>>BPCSHIFT)
>> ./linux-2.6/xfs_buf.c:  page_count = xfs_buf_btoc(end) - 
>> xfs_buf_btoct(bp->b_file_offset);
>> ./linux-2.6/xfs_buf.c:          page = bp->b_pages[xfs_buf_btoct(boff + 
>> bp->b_offset)];
>> ./linux-2.6/xfs_buf.h:#define xfs_buf_btoc(dd)  (((dd) + 
>> PAGE_CACHE_SIZE-1) >> PAGE_CACHE_SHIFT)
>> ./linux-2.6/xfs_buf.h:#define xfs_buf_btoct(dd) ((dd) >> PAGE_CACHE_SHIFT)
>>
>> --Tim
>>
>>> -Eric
>>>
>>>>  fs/xfs/linux-2.6/xfs_linux.h |    4 ----
>>>>  1 file changed, 4 deletions(-)
>>>>
>>>> --- a/fs/xfs/linux-2.6/xfs_linux.h    2007-11-07 11:26:20.000000000 
>>>> +0100
>>>> +++ b/fs/xfs/linux-2.6/xfs_linux.h    2007-11-11 13:07:11.000000000 
>>>> +0100
>>>> @@ -167,12 +167,8 @@
>>>>  
>>>>  /* 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 */
>>>>
>>>>
> So I'm thinking of "seeing" your patch and raising you by my attached patch :-)
> 
> --Tim
> 
> 
> -- Binary/unsupported file stripped by Ecartis --
> -- Type: text/x-patch
> -- File: btoc.patch
> 
> 
> 

Where the attachment was supposed to look like...

=====================================Index: fs/xfs/linux-2.6/xfs_linux.h
=====================================
--- a/fs/xfs/linux-2.6/xfs_linux.h	2007-11-12 11:24:05.000000000 +1100
+++ b/fs/xfs/linux-2.6/xfs_linux.h	2007-11-12 11:14:22.818831666 +1100
@@ -159,12 +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)
@@ -172,14 +166,6 @@
  /* 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 */

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

[David Chinner]

On Mon, Nov 12, 2007 at 11:33:30AM +1100, Timothy Shimmin wrote:
> Where the attachment was supposed to look like...
> 
> =====================================> Index: fs/xfs/linux-2.6/xfs_linux.h
> =====================================> 
> --- a/fs/xfs/linux-2.6/xfs_linux.h	2007-11-12 11:24:05.000000000 +1100
> +++ b/fs/xfs/linux-2.6/xfs_linux.h	2007-11-12 11:14:22.818831666 +1100
> @@ -159,12 +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)
> @@ -172,14 +166,6 @@
>   /* 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 */

Perhaps we should look at cleaning up the cusers of offtoc, offtoct, etc
and killing BPCSHIFT altogether....

Cheers,

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

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

[Timothy Shimmin]

David Chinner wrote:
> On Mon, Nov 12, 2007 at 11:33:30AM +1100, Timothy Shimmin wrote:
>> Where the attachment was supposed to look like...
>>
>> =====================================>> Index: fs/xfs/linux-2.6/xfs_linux.h
>> =====================================>>
>> --- a/fs/xfs/linux-2.6/xfs_linux.h	2007-11-12 11:24:05.000000000 +1100
>> +++ b/fs/xfs/linux-2.6/xfs_linux.h	2007-11-12 11:14:22.818831666 +1100
>> @@ -159,12 +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)
>> @@ -172,14 +166,6 @@
>>   /* 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 */
> 
> 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?

--Tim

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

[David Chinner]

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)

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

Cheers,

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

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

[Timothy Shimmin]

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@sgi.com>
---
  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;
  	}

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

[David Chinner]

On Wed, Nov 14, 2007 at 01:46:52PM +1100, Timothy Shimmin wrote:
> David Chinner wrote:
> > Just replace them with (val & PAGE_CACHE_MASK)
                                  ^^^^^^^^^^^^^^^
......
> --- 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),
                                                                  ^^^^^^^^^

s/PAGE_MASK/PAGE_CACHE_MASK/g

Cheers,

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

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

[Timothy Shimmin]

David Chinner wrote:
> On Wed, Nov 14, 2007 at 01:46:52PM +1100, Timothy Shimmin wrote:
>> David Chinner wrote:
>>> Just replace them with (val & PAGE_CACHE_MASK)
>                                   ^^^^^^^^^^^^^^^
> ......
>> --- 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),
>                                                                   ^^^^^^^^^
> 
> s/PAGE_MASK/PAGE_CACHE_MASK/g
> 

Okay.

While here, looking at a few others...

I'll get rid of BPCSHIFT.

And then...

#define NBPP            PAGE_SIZE
#define NDPP            (1 << (PAGE_SHIFT - 9)) <--- not used - another to nuke
#define NBPC            PAGE_SIZE <----- used once

grep -Ir 'NBPC' . | egrep -v 'tag|anot|diff'
./linux-2.6/xfs_linux.h:#define NBPC            PAGE_SIZE       /* Number of bytes per click */
./xfs_itable.c: irbuf = kmem_zalloc_greedy(&irbsize, NBPC, NBPC * 4,

 > grep -Ir 'NBPP' . | egrep -v 'tag|anot|diff|NBPPR'
./linux-2.6/xfs_linux.h:#define NBPP            PAGE_SIZE
./quota/xfs_qm.h:#define XFS_QM_HASHSIZE_LOW            (NBPP / sizeof(xfs_dqhash_t))
./quota/xfs_qm.h:#define XFS_QM_HASHSIZE_HIGH           ((NBPP * 4) / sizeof(xfs_dqhash_t))
./xfs_bmap.c:   } else if (mp->m_sb.sb_blocksize >= NBPP) {
./xfs_bmap.c:           args.prod = NBPP >> mp->m_sb.sb_blocklog;
./xfs_itable.c: bcount = MIN(left, (int)(NBPP / sizeof(*buffer)));
./xfs_log.c:                    kmem_free(tic, NBPP);
./xfs_log.c:    uint            i = (NBPP / sizeof(xlog_ticket_t)) - 2;
./xfs_log.c:    buf = (xfs_caddr_t) kmem_zalloc(NBPP, KM_SLEEP);
./xfs_vnodeops.c:       rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, NBPP);

Might as well get rid of NBPC and replace by NBPP.

Is it just worth s/NBPC/PAGE_SIZE/g ?

Okay, the xfs_vnodeops.c one should be PAGE_CACHE_SIZE, then right?
How about the bmap ones?

I don't know if I want to keep asking questions ... ;-)

--Tim

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

[David Chinner]

On Thu, Nov 15, 2007 at 11:54:04AM +1100, Timothy Shimmin wrote:
> David Chinner wrote:
> > On Wed, Nov 14, 2007 at 01:46:52PM +1100, Timothy Shimmin wrote:
> >> David Chinner wrote:
> >>> Just replace them with (val & PAGE_CACHE_MASK)
> >                                   ^^^^^^^^^^^^^^^
> > ......
> >> --- 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),
> >                                                                   ^^^^^^^^^
> > 
> > s/PAGE_MASK/PAGE_CACHE_MASK/g
> > 
> 
> Okay.
> 
> While here, looking at a few others...
> 
> I'll get rid of BPCSHIFT.
> 
> And then...
> 
> #define NBPP            PAGE_SIZE
> #define NDPP            (1 << (PAGE_SHIFT - 9)) <--- not used - another to nuke
> #define NBPC            PAGE_SIZE <----- used once
> 
> grep -Ir 'NBPC' . | egrep -v 'tag|anot|diff'
> ./linux-2.6/xfs_linux.h:#define NBPC            PAGE_SIZE       /* Number of bytes per click */
> ./xfs_itable.c: irbuf = kmem_zalloc_greedy(&irbsize, NBPC, NBPC * 4,
> 
>  > grep -Ir 'NBPP' . | egrep -v 'tag|anot|diff|NBPPR'
> ./linux-2.6/xfs_linux.h:#define NBPP            PAGE_SIZE
> ./quota/xfs_qm.h:#define XFS_QM_HASHSIZE_LOW            (NBPP / sizeof(xfs_dqhash_t))
> ./quota/xfs_qm.h:#define XFS_QM_HASHSIZE_HIGH           ((NBPP * 4) / sizeof(xfs_dqhash_t))

PAGE_SIZE

> ./xfs_bmap.c:   } else if (mp->m_sb.sb_blocksize >= NBPP) {
> ./xfs_bmap.c:           args.prod = NBPP >> mp->m_sb.sb_blocklog;

PAGE_CACHE_SIZE

> ./xfs_itable.c: bcount = MIN(left, (int)(NBPP / sizeof(*buffer)));

PAGE_SIZE

> ./xfs_log.c:                    kmem_free(tic, NBPP);
> ./xfs_log.c:    uint            i = (NBPP / sizeof(xlog_ticket_t)) - 2;
> ./xfs_log.c:    buf = (xfs_caddr_t) kmem_zalloc(NBPP, KM_SLEEP);

We should replace that hand rolled ticket allocator with a slab cache.

> ./xfs_vnodeops.c:       rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, NBPP);

PAGE_CACHE_SIZE. (as suggested ;)

> Might as well get rid of NBPC and replace by NBPP.
> 
> Is it just worth s/NBPC/PAGE_SIZE/g ?

yup.

Cheers,

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

[PATCH] remove BPCSHIFT and NB?P? macros - was fs/xfs: remove duplicated

[Timothy Shimmin]

David Chinner wrote:
>> While here, looking at a few others...
>> #define NBPP            PAGE_SIZE
>> #define NDPP            (1 << (PAGE_SHIFT - 9)) <--- not used - another to nuke
>> #define NBPC            PAGE_SIZE <----- used once
>>
>> grep -Ir 'NBPC' . | egrep -v 'tag|anot|diff'
>> ./linux-2.6/xfs_linux.h:#define NBPC            PAGE_SIZE       /* Number of bytes per click */
>> ./xfs_itable.c: irbuf = kmem_zalloc_greedy(&irbsize, NBPC, NBPC * 4,
>>
>>  > grep -Ir 'NBPP' . | egrep -v 'tag|anot|diff|NBPPR'
>> ./linux-2.6/xfs_linux.h:#define NBPP            PAGE_SIZE
>> ./quota/xfs_qm.h:#define XFS_QM_HASHSIZE_LOW            (NBPP / sizeof(xfs_dqhash_t))
>> ./quota/xfs_qm.h:#define XFS_QM_HASHSIZE_HIGH           ((NBPP * 4) / sizeof(xfs_dqhash_t))
> 
> PAGE_SIZE
> 
>> ./xfs_bmap.c:   } else if (mp->m_sb.sb_blocksize >= NBPP) {
>> ./xfs_bmap.c:           args.prod = NBPP >> mp->m_sb.sb_blocklog;
> 
> PAGE_CACHE_SIZE
> 
>> ./xfs_itable.c: bcount = MIN(left, (int)(NBPP / sizeof(*buffer)));
> 
> PAGE_SIZE
> 
>> ./xfs_log.c:                    kmem_free(tic, NBPP);
>> ./xfs_log.c:    uint            i = (NBPP / sizeof(xlog_ticket_t)) - 2;
>> ./xfs_log.c:    buf = (xfs_caddr_t) kmem_zalloc(NBPP, KM_SLEEP);
> 
> We should replace that hand rolled ticket allocator with a slab cache.
> 
In another patch though.

>> ./xfs_vnodeops.c:       rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, NBPP);
> 
> PAGE_CACHE_SIZE. (as suggested ;)
> 
>> Might as well get rid of NBPC and replace by NBPP.
>>
>> Is it just worth s/NBPC/PAGE_SIZE/g ?
> 
> yup.
> 
Thanks.

Patch:
-------------

Remove the BPCSHIFT and NB* based macros from XFS.

The BPCSHIFT based macros, btoc*, ctob*, offtoc* and ctooff
are either not used or don't need to be used.
The NDPP, NDPP, NBBY macros don't need to be used but instead
are replaced directly by PAGE_SIZE and PAGE_CACHE_SIZE
where appropriate.
Initial patch and motivation from Nicolas Kaiser.

Signed-Off-By: Tim Shimmin <tes@sgi.com>
---
  b/fs/xfs/linux-2.6/xfs_linux.h |   28 +---------------------------
  b/fs/xfs/linux-2.6/xfs_lrw.c   |    9 ++++-----
  b/fs/xfs/quota/xfs_qm.h        |    4 ++--
  b/fs/xfs/xfs_bmap.c            |    4 ++--
  b/fs/xfs/xfs_itable.c          |    4 ++--
  b/fs/xfs/xfs_log.c             |    6 +++---
  b/fs/xfs/xfs_vnodeops.c        |    9 +++------
  7 files changed, 17 insertions(+), 47 deletions(-)

=====================================Index: fs/xfs/linux-2.6/xfs_linux.h
=====================================
--- a/fs/xfs/linux-2.6/xfs_linux.h	2007-11-16 16:22:40.000000000 +1100
+++ b/fs/xfs/linux-2.6/xfs_linux.h	2007-11-16 15:59:22.299466117 +1100
@@ -143,43 +143,17 @@

  #define spinlock_destroy(lock)

-#define NBPP		PAGE_SIZE
-#define NDPP		(1 << (PAGE_SHIFT - 9))
-
  #define NBBY		8		/* number of bits per byte */
-#define	NBPC		PAGE_SIZE	/* Number of bytes per click */
-#define	BPCSHIFT	PAGE_SHIFT	/* LOG2(NBPC) if exact */

  /*
   * Size of block device i/o is parameterized here.
   * Currently the system supports page-sized i/o.
   */
-#define	BLKDEV_IOSHIFT		BPCSHIFT
+#define	BLKDEV_IOSHIFT		PAGE_CACHE_SHIFT
  #define	BLKDEV_IOSIZE		(1<<BLKDEV_IOSHIFT)
  /* 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-16 16:22:40.000000000 +1100
+++ b/fs/xfs/linux-2.6/xfs_lrw.c	2007-11-15 11:32:12.451249159 +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_CACHE_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_CACHE_MASK), -1);
  			error = xfs_flushinval_pages(xip,
-					ctooff(offtoct(pos)),
+					(pos & PAGE_CACHE_MASK),
  					-1, FI_REMAPF_LOCKED);
  			if (error)
  				goto out_unlock_internal;

=====================================Index: fs/xfs/quota/xfs_qm.h
=====================================
--- a/fs/xfs/quota/xfs_qm.h	2007-11-16 16:22:40.000000000 +1100
+++ b/fs/xfs/quota/xfs_qm.h	2007-11-16 15:39:53.373375739 +1100
@@ -52,8 +52,8 @@ extern kmem_zone_t	*qm_dqtrxzone;
  /*
   * Dquot hashtable constants/threshold values.
   */
-#define XFS_QM_HASHSIZE_LOW		(NBPP / sizeof(xfs_dqhash_t))
-#define XFS_QM_HASHSIZE_HIGH		((NBPP * 4) / sizeof(xfs_dqhash_t))
+#define XFS_QM_HASHSIZE_LOW		(PAGE_SIZE / sizeof(xfs_dqhash_t))
+#define XFS_QM_HASHSIZE_HIGH		((PAGE_SIZE * 4) / sizeof(xfs_dqhash_t))

  /*
   * This defines the unit of allocation of dquots.

=====================================Index: fs/xfs/xfs_bmap.c
=====================================
--- a/fs/xfs/xfs_bmap.c	2007-11-16 16:22:41.000000000 +1100
+++ b/fs/xfs/xfs_bmap.c	2007-11-16 15:40:27.017050666 +1100
@@ -2830,11 +2830,11 @@ xfs_bmap_btalloc(
  		args.prod = align;
  		if ((args.mod = (xfs_extlen_t)do_mod(ap->off, args.prod)))
  			args.mod = (xfs_extlen_t)(args.prod - args.mod);
-	} else if (mp->m_sb.sb_blocksize >= NBPP) {
+	} else if (mp->m_sb.sb_blocksize >= PAGE_CACHE_SIZE) {
  		args.prod = 1;
  		args.mod = 0;
  	} else {
-		args.prod = NBPP >> mp->m_sb.sb_blocklog;
+		args.prod = PAGE_CACHE_SIZE >> mp->m_sb.sb_blocklog;
  		if ((args.mod = (xfs_extlen_t)(do_mod(ap->off, args.prod))))
  			args.mod = (xfs_extlen_t)(args.prod - args.mod);
  	}

=====================================Index: fs/xfs/xfs_itable.c
=====================================
--- a/fs/xfs/xfs_itable.c	2007-11-16 16:22:40.000000000 +1100
+++ b/fs/xfs/xfs_itable.c	2007-11-16 16:05:51.381600461 +1100
@@ -393,7 +393,7 @@ xfs_bulkstat(
  		(XFS_INODE_CLUSTER_SIZE(mp) >> mp->m_sb.sb_inodelog);
  	nimask = ~(nicluster - 1);
  	nbcluster = nicluster >> mp->m_sb.sb_inopblog;
-	irbuf = kmem_zalloc_greedy(&irbsize, NBPC, NBPC * 4,
+	irbuf = kmem_zalloc_greedy(&irbsize, PAGE_SIZE, PAGE_SIZE * 4,
  				   KM_SLEEP | KM_MAYFAIL | KM_LARGE);
  	nirbuf = irbsize / sizeof(*irbuf);

@@ -815,7 +815,7 @@ xfs_inumbers(
  	agino = XFS_INO_TO_AGINO(mp, ino);
  	left = *count;
  	*count = 0;
-	bcount = MIN(left, (int)(NBPP / sizeof(*buffer)));
+	bcount = MIN(left, (int)(PAGE_SIZE / sizeof(*buffer)));
  	buffer = kmem_alloc(bcount * sizeof(*buffer), KM_SLEEP);
  	error = bufidx = 0;
  	cur = NULL;

=====================================Index: fs/xfs/xfs_log.c
=====================================
--- a/fs/xfs/xfs_log.c	2007-11-16 16:22:40.000000000 +1100
+++ b/fs/xfs/xfs_log.c	2007-11-16 15:56:42.615917720 +1100
@@ -1552,7 +1552,7 @@ xlog_dealloc_log(xlog_t *log)
  		tic = log->l_unmount_free;
  		while (tic) {
  			next_tic = tic->t_next;
-			kmem_free(tic, NBPP);
+			kmem_free(tic, PAGE_SIZE);
  			tic = next_tic;
  		}
  	}
@@ -3161,13 +3161,13 @@ xlog_state_ticket_alloc(xlog_t *log)
  	xlog_ticket_t	*t_list;
  	xlog_ticket_t	*next;
  	xfs_caddr_t	buf;
-	uint		i = (NBPP / sizeof(xlog_ticket_t)) - 2;
+	uint		i = (PAGE_SIZE / sizeof(xlog_ticket_t)) - 2;

  	/*
  	 * The kmem_zalloc may sleep, so we shouldn't be holding the
  	 * global lock.  XXXmiken: may want to use zone allocator.
  	 */
-	buf = (xfs_caddr_t) kmem_zalloc(NBPP, KM_SLEEP);
+	buf = (xfs_caddr_t) kmem_zalloc(PAGE_SIZE, KM_SLEEP);

  	spin_lock(&log->l_icloglock);


=====================================Index: fs/xfs/xfs_vnodeops.c
=====================================
--- a/fs/xfs/xfs_vnodeops.c	2007-11-16 16:22:41.000000000 +1100
+++ b/fs/xfs/xfs_vnodeops.c	2007-11-16 15:57:01.449505791 +1100
@@ -4164,15 +4164,12 @@ xfs_free_file_space(
  		vn_iowait(ip);	/* wait for the completion of any pending DIOs */
  	}

-	rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, NBPP);
+	rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
  	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;
  	}

Re: [xfs-masters] [PATCH] remove BPCSHIFT and NB?P? macros - was

[Lachlan McIlroy]

Looks good Tim.  Nice work.

Timothy Shimmin wrote:
> David Chinner wrote:
>>> While here, looking at a few others...
>>> #define NBPP            PAGE_SIZE
>>> #define NDPP            (1 << (PAGE_SHIFT - 9)) <--- not used - another to nuke
>>> #define NBPC            PAGE_SIZE <----- used once
>>>
>>> grep -Ir 'NBPC' . | egrep -v 'tag|anot|diff'
>>> ./linux-2.6/xfs_linux.h:#define NBPC            PAGE_SIZE       /* Number of bytes per click */
>>> ./xfs_itable.c: irbuf = kmem_zalloc_greedy(&irbsize, NBPC, NBPC * 4,
>>>
>>>  > grep -Ir 'NBPP' . | egrep -v 'tag|anot|diff|NBPPR'
>>> ./linux-2.6/xfs_linux.h:#define NBPP            PAGE_SIZE
>>> ./quota/xfs_qm.h:#define XFS_QM_HASHSIZE_LOW            (NBPP / sizeof(xfs_dqhash_t))
>>> ./quota/xfs_qm.h:#define XFS_QM_HASHSIZE_HIGH           ((NBPP * 4) / sizeof(xfs_dqhash_t))
>> PAGE_SIZE
>>
>>> ./xfs_bmap.c:   } else if (mp->m_sb.sb_blocksize >= NBPP) {
>>> ./xfs_bmap.c:           args.prod = NBPP >> mp->m_sb.sb_blocklog;
>> PAGE_CACHE_SIZE
>>
>>> ./xfs_itable.c: bcount = MIN(left, (int)(NBPP / sizeof(*buffer)));
>> PAGE_SIZE
>>
>>> ./xfs_log.c:                    kmem_free(tic, NBPP);
>>> ./xfs_log.c:    uint            i = (NBPP / sizeof(xlog_ticket_t)) - 2;
>>> ./xfs_log.c:    buf = (xfs_caddr_t) kmem_zalloc(NBPP, KM_SLEEP);
>> We should replace that hand rolled ticket allocator with a slab cache.
>>
> In another patch though.
> 
>>> ./xfs_vnodeops.c:       rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, NBPP);
>> PAGE_CACHE_SIZE. (as suggested ;)
>>
>>> Might as well get rid of NBPC and replace by NBPP.
>>>
>>> Is it just worth s/NBPC/PAGE_SIZE/g ?
>> yup.
>>
> Thanks.
> 
> Patch:
> -------------
> 
> Remove the BPCSHIFT and NB* based macros from XFS.
> 
> The BPCSHIFT based macros, btoc*, ctob*, offtoc* and ctooff
> are either not used or don't need to be used.
> The NDPP, NDPP, NBBY macros don't need to be used but instead
> are replaced directly by PAGE_SIZE and PAGE_CACHE_SIZE
> where appropriate.
> Initial patch and motivation from Nicolas Kaiser.
> 
> Signed-Off-By: Tim Shimmin <tes@sgi.com>
> ---
>   b/fs/xfs/linux-2.6/xfs_linux.h |   28 +---------------------------
>   b/fs/xfs/linux-2.6/xfs_lrw.c   |    9 ++++-----
>   b/fs/xfs/quota/xfs_qm.h        |    4 ++--
>   b/fs/xfs/xfs_bmap.c            |    4 ++--
>   b/fs/xfs/xfs_itable.c          |    4 ++--
>   b/fs/xfs/xfs_log.c             |    6 +++---
>   b/fs/xfs/xfs_vnodeops.c        |    9 +++------
>   7 files changed, 17 insertions(+), 47 deletions(-)
> 
> =====================================> Index: fs/xfs/linux-2.6/xfs_linux.h
> =====================================> 
> --- a/fs/xfs/linux-2.6/xfs_linux.h	2007-11-16 16:22:40.000000000 +1100
> +++ b/fs/xfs/linux-2.6/xfs_linux.h	2007-11-16 15:59:22.299466117 +1100
> @@ -143,43 +143,17 @@
> 
>   #define spinlock_destroy(lock)
> 
> -#define NBPP		PAGE_SIZE
> -#define NDPP		(1 << (PAGE_SHIFT - 9))
> -
>   #define NBBY		8		/* number of bits per byte */
> -#define	NBPC		PAGE_SIZE	/* Number of bytes per click */
> -#define	BPCSHIFT	PAGE_SHIFT	/* LOG2(NBPC) if exact */
> 
>   /*
>    * Size of block device i/o is parameterized here.
>    * Currently the system supports page-sized i/o.
>    */
> -#define	BLKDEV_IOSHIFT		BPCSHIFT
> +#define	BLKDEV_IOSHIFT		PAGE_CACHE_SHIFT
>   #define	BLKDEV_IOSIZE		(1<<BLKDEV_IOSHIFT)
>   /* 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-16 16:22:40.000000000 +1100
> +++ b/fs/xfs/linux-2.6/xfs_lrw.c	2007-11-15 11:32:12.451249159 +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_CACHE_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_CACHE_MASK), -1);
>   			error = xfs_flushinval_pages(xip,
> -					ctooff(offtoct(pos)),
> +					(pos & PAGE_CACHE_MASK),
>   					-1, FI_REMAPF_LOCKED);
>   			if (error)
>   				goto out_unlock_internal;
> 
> =====================================> Index: fs/xfs/quota/xfs_qm.h
> =====================================> 
> --- a/fs/xfs/quota/xfs_qm.h	2007-11-16 16:22:40.000000000 +1100
> +++ b/fs/xfs/quota/xfs_qm.h	2007-11-16 15:39:53.373375739 +1100
> @@ -52,8 +52,8 @@ extern kmem_zone_t	*qm_dqtrxzone;
>   /*
>    * Dquot hashtable constants/threshold values.
>    */
> -#define XFS_QM_HASHSIZE_LOW		(NBPP / sizeof(xfs_dqhash_t))
> -#define XFS_QM_HASHSIZE_HIGH		((NBPP * 4) / sizeof(xfs_dqhash_t))
> +#define XFS_QM_HASHSIZE_LOW		(PAGE_SIZE / sizeof(xfs_dqhash_t))
> +#define XFS_QM_HASHSIZE_HIGH		((PAGE_SIZE * 4) / sizeof(xfs_dqhash_t))
> 
>   /*
>    * This defines the unit of allocation of dquots.
> 
> =====================================> Index: fs/xfs/xfs_bmap.c
> =====================================> 
> --- a/fs/xfs/xfs_bmap.c	2007-11-16 16:22:41.000000000 +1100
> +++ b/fs/xfs/xfs_bmap.c	2007-11-16 15:40:27.017050666 +1100
> @@ -2830,11 +2830,11 @@ xfs_bmap_btalloc(
>   		args.prod = align;
>   		if ((args.mod = (xfs_extlen_t)do_mod(ap->off, args.prod)))
>   			args.mod = (xfs_extlen_t)(args.prod - args.mod);
> -	} else if (mp->m_sb.sb_blocksize >= NBPP) {
> +	} else if (mp->m_sb.sb_blocksize >= PAGE_CACHE_SIZE) {
>   		args.prod = 1;
>   		args.mod = 0;
>   	} else {
> -		args.prod = NBPP >> mp->m_sb.sb_blocklog;
> +		args.prod = PAGE_CACHE_SIZE >> mp->m_sb.sb_blocklog;
>   		if ((args.mod = (xfs_extlen_t)(do_mod(ap->off, args.prod))))
>   			args.mod = (xfs_extlen_t)(args.prod - args.mod);
>   	}
> 
> =====================================> Index: fs/xfs/xfs_itable.c
> =====================================> 
> --- a/fs/xfs/xfs_itable.c	2007-11-16 16:22:40.000000000 +1100
> +++ b/fs/xfs/xfs_itable.c	2007-11-16 16:05:51.381600461 +1100
> @@ -393,7 +393,7 @@ xfs_bulkstat(
>   		(XFS_INODE_CLUSTER_SIZE(mp) >> mp->m_sb.sb_inodelog);
>   	nimask = ~(nicluster - 1);
>   	nbcluster = nicluster >> mp->m_sb.sb_inopblog;
> -	irbuf = kmem_zalloc_greedy(&irbsize, NBPC, NBPC * 4,
> +	irbuf = kmem_zalloc_greedy(&irbsize, PAGE_SIZE, PAGE_SIZE * 4,
>   				   KM_SLEEP | KM_MAYFAIL | KM_LARGE);
>   	nirbuf = irbsize / sizeof(*irbuf);
> 
> @@ -815,7 +815,7 @@ xfs_inumbers(
>   	agino = XFS_INO_TO_AGINO(mp, ino);
>   	left = *count;
>   	*count = 0;
> -	bcount = MIN(left, (int)(NBPP / sizeof(*buffer)));
> +	bcount = MIN(left, (int)(PAGE_SIZE / sizeof(*buffer)));
>   	buffer = kmem_alloc(bcount * sizeof(*buffer), KM_SLEEP);
>   	error = bufidx = 0;
>   	cur = NULL;
> 
> =====================================> Index: fs/xfs/xfs_log.c
> =====================================> 
> --- a/fs/xfs/xfs_log.c	2007-11-16 16:22:40.000000000 +1100
> +++ b/fs/xfs/xfs_log.c	2007-11-16 15:56:42.615917720 +1100
> @@ -1552,7 +1552,7 @@ xlog_dealloc_log(xlog_t *log)
>   		tic = log->l_unmount_free;
>   		while (tic) {
>   			next_tic = tic->t_next;
> -			kmem_free(tic, NBPP);
> +			kmem_free(tic, PAGE_SIZE);
>   			tic = next_tic;
>   		}
>   	}
> @@ -3161,13 +3161,13 @@ xlog_state_ticket_alloc(xlog_t *log)
>   	xlog_ticket_t	*t_list;
>   	xlog_ticket_t	*next;
>   	xfs_caddr_t	buf;
> -	uint		i = (NBPP / sizeof(xlog_ticket_t)) - 2;
> +	uint		i = (PAGE_SIZE / sizeof(xlog_ticket_t)) - 2;
> 
>   	/*
>   	 * The kmem_zalloc may sleep, so we shouldn't be holding the
>   	 * global lock.  XXXmiken: may want to use zone allocator.
>   	 */
> -	buf = (xfs_caddr_t) kmem_zalloc(NBPP, KM_SLEEP);
> +	buf = (xfs_caddr_t) kmem_zalloc(PAGE_SIZE, KM_SLEEP);
> 
>   	spin_lock(&log->l_icloglock);
> 
> 
> =====================================> Index: fs/xfs/xfs_vnodeops.c
> =====================================> 
> --- a/fs/xfs/xfs_vnodeops.c	2007-11-16 16:22:41.000000000 +1100
> +++ b/fs/xfs/xfs_vnodeops.c	2007-11-16 15:57:01.449505791 +1100
> @@ -4164,15 +4164,12 @@ xfs_free_file_space(
>   		vn_iowait(ip);	/* wait for the completion of any pending DIOs */
>   	}
> 
> -	rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, NBPP);
> +	rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
>   	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;
>   	}
> 
> 
> 
> 

Re: [xfs-masters] [PATCH] remove BPCSHIFT and NB?P? macros - was fs/xfs: remove duplicated defines

[Lachlan McIlroy]

Looks good Tim.  Nice work.

Timothy Shimmin wrote:
> David Chinner wrote:
>>> While here, looking at a few others...
>>> #define NBPP            PAGE_SIZE
>>> #define NDPP            (1 << (PAGE_SHIFT - 9)) <--- not used - another to nuke
>>> #define NBPC            PAGE_SIZE <----- used once
>>>
>>> grep -Ir 'NBPC' . | egrep -v 'tag|anot|diff'
>>> ./linux-2.6/xfs_linux.h:#define NBPC            PAGE_SIZE       /* Number of bytes per click */
>>> ./xfs_itable.c: irbuf = kmem_zalloc_greedy(&irbsize, NBPC, NBPC * 4,
>>>
>>>  > grep -Ir 'NBPP' . | egrep -v 'tag|anot|diff|NBPPR'
>>> ./linux-2.6/xfs_linux.h:#define NBPP            PAGE_SIZE
>>> ./quota/xfs_qm.h:#define XFS_QM_HASHSIZE_LOW            (NBPP / sizeof(xfs_dqhash_t))
>>> ./quota/xfs_qm.h:#define XFS_QM_HASHSIZE_HIGH           ((NBPP * 4) / sizeof(xfs_dqhash_t))
>> PAGE_SIZE
>>
>>> ./xfs_bmap.c:   } else if (mp->m_sb.sb_blocksize >= NBPP) {
>>> ./xfs_bmap.c:           args.prod = NBPP >> mp->m_sb.sb_blocklog;
>> PAGE_CACHE_SIZE
>>
>>> ./xfs_itable.c: bcount = MIN(left, (int)(NBPP / sizeof(*buffer)));
>> PAGE_SIZE
>>
>>> ./xfs_log.c:                    kmem_free(tic, NBPP);
>>> ./xfs_log.c:    uint            i = (NBPP / sizeof(xlog_ticket_t)) - 2;
>>> ./xfs_log.c:    buf = (xfs_caddr_t) kmem_zalloc(NBPP, KM_SLEEP);
>> We should replace that hand rolled ticket allocator with a slab cache.
>>
> In another patch though.
> 
>>> ./xfs_vnodeops.c:       rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, NBPP);
>> PAGE_CACHE_SIZE. (as suggested ;)
>>
>>> Might as well get rid of NBPC and replace by NBPP.
>>>
>>> Is it just worth s/NBPC/PAGE_SIZE/g ?
>> yup.
>>
> Thanks.
> 
> Patch:
> -------------
> 
> Remove the BPCSHIFT and NB* based macros from XFS.
> 
> The BPCSHIFT based macros, btoc*, ctob*, offtoc* and ctooff
> are either not used or don't need to be used.
> The NDPP, NDPP, NBBY macros don't need to be used but instead
> are replaced directly by PAGE_SIZE and PAGE_CACHE_SIZE
> where appropriate.
> Initial patch and motivation from Nicolas Kaiser.
> 
> Signed-Off-By: Tim Shimmin <tes@sgi.com>
> ---
>   b/fs/xfs/linux-2.6/xfs_linux.h |   28 +---------------------------
>   b/fs/xfs/linux-2.6/xfs_lrw.c   |    9 ++++-----
>   b/fs/xfs/quota/xfs_qm.h        |    4 ++--
>   b/fs/xfs/xfs_bmap.c            |    4 ++--
>   b/fs/xfs/xfs_itable.c          |    4 ++--
>   b/fs/xfs/xfs_log.c             |    6 +++---
>   b/fs/xfs/xfs_vnodeops.c        |    9 +++------
>   7 files changed, 17 insertions(+), 47 deletions(-)
> 
> ===========================================================================
> Index: fs/xfs/linux-2.6/xfs_linux.h
> ===========================================================================
> 
> --- a/fs/xfs/linux-2.6/xfs_linux.h	2007-11-16 16:22:40.000000000 +1100
> +++ b/fs/xfs/linux-2.6/xfs_linux.h	2007-11-16 15:59:22.299466117 +1100
> @@ -143,43 +143,17 @@
> 
>   #define spinlock_destroy(lock)
> 
> -#define NBPP		PAGE_SIZE
> -#define NDPP		(1 << (PAGE_SHIFT - 9))
> -
>   #define NBBY		8		/* number of bits per byte */
> -#define	NBPC		PAGE_SIZE	/* Number of bytes per click */
> -#define	BPCSHIFT	PAGE_SHIFT	/* LOG2(NBPC) if exact */
> 
>   /*
>    * Size of block device i/o is parameterized here.
>    * Currently the system supports page-sized i/o.
>    */
> -#define	BLKDEV_IOSHIFT		BPCSHIFT
> +#define	BLKDEV_IOSHIFT		PAGE_CACHE_SHIFT
>   #define	BLKDEV_IOSIZE		(1<<BLKDEV_IOSHIFT)
>   /* 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-16 16:22:40.000000000 +1100
> +++ b/fs/xfs/linux-2.6/xfs_lrw.c	2007-11-15 11:32:12.451249159 +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_CACHE_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_CACHE_MASK), -1);
>   			error = xfs_flushinval_pages(xip,
> -					ctooff(offtoct(pos)),
> +					(pos & PAGE_CACHE_MASK),
>   					-1, FI_REMAPF_LOCKED);
>   			if (error)
>   				goto out_unlock_internal;
> 
> ===========================================================================
> Index: fs/xfs/quota/xfs_qm.h
> ===========================================================================
> 
> --- a/fs/xfs/quota/xfs_qm.h	2007-11-16 16:22:40.000000000 +1100
> +++ b/fs/xfs/quota/xfs_qm.h	2007-11-16 15:39:53.373375739 +1100
> @@ -52,8 +52,8 @@ extern kmem_zone_t	*qm_dqtrxzone;
>   /*
>    * Dquot hashtable constants/threshold values.
>    */
> -#define XFS_QM_HASHSIZE_LOW		(NBPP / sizeof(xfs_dqhash_t))
> -#define XFS_QM_HASHSIZE_HIGH		((NBPP * 4) / sizeof(xfs_dqhash_t))
> +#define XFS_QM_HASHSIZE_LOW		(PAGE_SIZE / sizeof(xfs_dqhash_t))
> +#define XFS_QM_HASHSIZE_HIGH		((PAGE_SIZE * 4) / sizeof(xfs_dqhash_t))
> 
>   /*
>    * This defines the unit of allocation of dquots.
> 
> ===========================================================================
> Index: fs/xfs/xfs_bmap.c
> ===========================================================================
> 
> --- a/fs/xfs/xfs_bmap.c	2007-11-16 16:22:41.000000000 +1100
> +++ b/fs/xfs/xfs_bmap.c	2007-11-16 15:40:27.017050666 +1100
> @@ -2830,11 +2830,11 @@ xfs_bmap_btalloc(
>   		args.prod = align;
>   		if ((args.mod = (xfs_extlen_t)do_mod(ap->off, args.prod)))
>   			args.mod = (xfs_extlen_t)(args.prod - args.mod);
> -	} else if (mp->m_sb.sb_blocksize >= NBPP) {
> +	} else if (mp->m_sb.sb_blocksize >= PAGE_CACHE_SIZE) {
>   		args.prod = 1;
>   		args.mod = 0;
>   	} else {
> -		args.prod = NBPP >> mp->m_sb.sb_blocklog;
> +		args.prod = PAGE_CACHE_SIZE >> mp->m_sb.sb_blocklog;
>   		if ((args.mod = (xfs_extlen_t)(do_mod(ap->off, args.prod))))
>   			args.mod = (xfs_extlen_t)(args.prod - args.mod);
>   	}
> 
> ===========================================================================
> Index: fs/xfs/xfs_itable.c
> ===========================================================================
> 
> --- a/fs/xfs/xfs_itable.c	2007-11-16 16:22:40.000000000 +1100
> +++ b/fs/xfs/xfs_itable.c	2007-11-16 16:05:51.381600461 +1100
> @@ -393,7 +393,7 @@ xfs_bulkstat(
>   		(XFS_INODE_CLUSTER_SIZE(mp) >> mp->m_sb.sb_inodelog);
>   	nimask = ~(nicluster - 1);
>   	nbcluster = nicluster >> mp->m_sb.sb_inopblog;
> -	irbuf = kmem_zalloc_greedy(&irbsize, NBPC, NBPC * 4,
> +	irbuf = kmem_zalloc_greedy(&irbsize, PAGE_SIZE, PAGE_SIZE * 4,
>   				   KM_SLEEP | KM_MAYFAIL | KM_LARGE);
>   	nirbuf = irbsize / sizeof(*irbuf);
> 
> @@ -815,7 +815,7 @@ xfs_inumbers(
>   	agino = XFS_INO_TO_AGINO(mp, ino);
>   	left = *count;
>   	*count = 0;
> -	bcount = MIN(left, (int)(NBPP / sizeof(*buffer)));
> +	bcount = MIN(left, (int)(PAGE_SIZE / sizeof(*buffer)));
>   	buffer = kmem_alloc(bcount * sizeof(*buffer), KM_SLEEP);
>   	error = bufidx = 0;
>   	cur = NULL;
> 
> ===========================================================================
> Index: fs/xfs/xfs_log.c
> ===========================================================================
> 
> --- a/fs/xfs/xfs_log.c	2007-11-16 16:22:40.000000000 +1100
> +++ b/fs/xfs/xfs_log.c	2007-11-16 15:56:42.615917720 +1100
> @@ -1552,7 +1552,7 @@ xlog_dealloc_log(xlog_t *log)
>   		tic = log->l_unmount_free;
>   		while (tic) {
>   			next_tic = tic->t_next;
> -			kmem_free(tic, NBPP);
> +			kmem_free(tic, PAGE_SIZE);
>   			tic = next_tic;
>   		}
>   	}
> @@ -3161,13 +3161,13 @@ xlog_state_ticket_alloc(xlog_t *log)
>   	xlog_ticket_t	*t_list;
>   	xlog_ticket_t	*next;
>   	xfs_caddr_t	buf;
> -	uint		i = (NBPP / sizeof(xlog_ticket_t)) - 2;
> +	uint		i = (PAGE_SIZE / sizeof(xlog_ticket_t)) - 2;
> 
>   	/*
>   	 * The kmem_zalloc may sleep, so we shouldn't be holding the
>   	 * global lock.  XXXmiken: may want to use zone allocator.
>   	 */
> -	buf = (xfs_caddr_t) kmem_zalloc(NBPP, KM_SLEEP);
> +	buf = (xfs_caddr_t) kmem_zalloc(PAGE_SIZE, KM_SLEEP);
> 
>   	spin_lock(&log->l_icloglock);
> 
> 
> ===========================================================================
> Index: fs/xfs/xfs_vnodeops.c
> ===========================================================================
> 
> --- a/fs/xfs/xfs_vnodeops.c	2007-11-16 16:22:41.000000000 +1100
> +++ b/fs/xfs/xfs_vnodeops.c	2007-11-16 15:57:01.449505791 +1100
> @@ -4164,15 +4164,12 @@ xfs_free_file_space(
>   		vn_iowait(ip);	/* wait for the completion of any pending DIOs */
>   	}
> 
> -	rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, NBPP);
> +	rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
>   	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;
>   	}
> 
> 
> 
>