* Re: [xfs-masters] [PATCH] fs/xfs: remove duplicated defines
2007-11-11 12:43 [PATCH] fs/xfs: remove duplicated defines Nicolas Kaiser
@ 2007-11-11 23:57 ` Eric Sandeen
2007-11-12 0:28 ` Timothy Shimmin
` (8 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Eric Sandeen @ 2007-11-11 23:57 UTC (permalink / raw)
To: xfs-masters; +Cc: xfs, kernel-janitors
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 */
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [xfs-masters] [PATCH] fs/xfs: remove duplicated defines
@ 2007-11-11 23:57 ` Eric Sandeen
0 siblings, 0 replies; 17+ messages in thread
From: Eric Sandeen @ 2007-11-11 23:57 UTC (permalink / raw)
To: xfs-masters; +Cc: xfs, kernel-janitors
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 */
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [xfs-masters] [PATCH] fs/xfs: remove duplicated defines
2007-11-11 23:57 ` Eric Sandeen
@ 2007-11-12 0:09 ` Timothy Shimmin
-1 siblings, 0 replies; 17+ messages in thread
From: Timothy Shimmin @ 2007-11-12 0:09 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs-masters, xfs, kernel-janitors
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 */
>>
>>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [xfs-masters] [PATCH] fs/xfs: remove duplicated defines
@ 2007-11-12 0:09 ` Timothy Shimmin
0 siblings, 0 replies; 17+ messages in thread
From: Timothy Shimmin @ 2007-11-12 0:09 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs-masters, xfs, kernel-janitors
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 */
>>
>>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [xfs-masters] [PATCH] fs/xfs: remove duplicated defines
2007-11-11 12:43 [PATCH] fs/xfs: remove duplicated defines Nicolas Kaiser
2007-11-11 23:57 ` Eric Sandeen
@ 2007-11-12 0:28 ` Timothy Shimmin
2007-11-12 0:33 ` [xfs-masters] " Timothy Shimmin
` (7 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Timothy Shimmin @ 2007-11-12 0:28 UTC (permalink / raw)
To: kernel-janitors
[-- 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 */
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [xfs-masters] Re: [PATCH] fs/xfs: remove duplicated defines
2007-11-11 12:43 [PATCH] fs/xfs: remove duplicated defines Nicolas Kaiser
2007-11-11 23:57 ` Eric Sandeen
2007-11-12 0:28 ` Timothy Shimmin
@ 2007-11-12 0:33 ` Timothy Shimmin
2007-11-12 2:04 ` David Chinner
` (6 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Timothy Shimmin @ 2007-11-12 0:33 UTC (permalink / raw)
To: kernel-janitors
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 */
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [xfs-masters] Re: [PATCH] fs/xfs: remove duplicated defines
2007-11-11 12:43 [PATCH] fs/xfs: remove duplicated defines Nicolas Kaiser
` (2 preceding siblings ...)
2007-11-12 0:33 ` [xfs-masters] " Timothy Shimmin
@ 2007-11-12 2:04 ` David Chinner
2007-11-13 5:35 ` Timothy Shimmin
` (5 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: David Chinner @ 2007-11-12 2:04 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [xfs-masters] Re: [PATCH] fs/xfs: remove duplicated defines
2007-11-11 12:43 [PATCH] fs/xfs: remove duplicated defines Nicolas Kaiser
` (3 preceding siblings ...)
2007-11-12 2:04 ` David Chinner
@ 2007-11-13 5:35 ` Timothy Shimmin
2007-11-13 21:30 ` David Chinner
` (4 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Timothy Shimmin @ 2007-11-13 5:35 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [xfs-masters] Re: [PATCH] fs/xfs: remove duplicated defines
2007-11-11 12:43 [PATCH] fs/xfs: remove duplicated defines Nicolas Kaiser
` (4 preceding siblings ...)
2007-11-13 5:35 ` Timothy Shimmin
@ 2007-11-13 21:30 ` David Chinner
2007-11-14 2:46 ` Timothy Shimmin
` (3 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: David Chinner @ 2007-11-13 21:30 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [xfs-masters] Re: [PATCH] fs/xfs: remove duplicated defines
2007-11-11 12:43 [PATCH] fs/xfs: remove duplicated defines Nicolas Kaiser
` (5 preceding siblings ...)
2007-11-13 21:30 ` David Chinner
@ 2007-11-14 2:46 ` Timothy Shimmin
2007-11-14 5:41 ` David Chinner
` (2 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Timothy Shimmin @ 2007-11-14 2:46 UTC (permalink / raw)
To: kernel-janitors
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;
}
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [xfs-masters] Re: [PATCH] fs/xfs: remove duplicated defines
2007-11-11 12:43 [PATCH] fs/xfs: remove duplicated defines Nicolas Kaiser
` (6 preceding siblings ...)
2007-11-14 2:46 ` Timothy Shimmin
@ 2007-11-14 5:41 ` David Chinner
2007-11-15 0:54 ` Timothy Shimmin
2007-11-15 6:27 ` David Chinner
9 siblings, 0 replies; 17+ messages in thread
From: David Chinner @ 2007-11-14 5:41 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [xfs-masters] Re: [PATCH] fs/xfs: remove duplicated defines
2007-11-11 12:43 [PATCH] fs/xfs: remove duplicated defines Nicolas Kaiser
` (7 preceding siblings ...)
2007-11-14 5:41 ` David Chinner
@ 2007-11-15 0:54 ` Timothy Shimmin
2007-11-15 6:27 ` David Chinner
9 siblings, 0 replies; 17+ messages in thread
From: Timothy Shimmin @ 2007-11-15 0:54 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [xfs-masters] Re: [PATCH] fs/xfs: remove duplicated defines
2007-11-11 12:43 [PATCH] fs/xfs: remove duplicated defines Nicolas Kaiser
` (8 preceding siblings ...)
2007-11-15 0:54 ` Timothy Shimmin
@ 2007-11-15 6:27 ` David Chinner
2007-11-16 5:34 ` [PATCH] remove BPCSHIFT and NB?P? macros - was fs/xfs: remove duplicated Timothy Shimmin
9 siblings, 1 reply; 17+ messages in thread
From: David Chinner @ 2007-11-15 6:27 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH] remove BPCSHIFT and NB?P? macros - was fs/xfs: remove duplicated
@ 2007-11-16 5:34 ` Timothy Shimmin
2007-11-16 6:15 ` [xfs-masters] [PATCH] remove BPCSHIFT and NB?P? macros - was fs/xfs: remove duplicated defines Lachlan McIlroy
0 siblings, 1 reply; 17+ messages in thread
From: Timothy Shimmin @ 2007-11-16 5:34 UTC (permalink / raw)
To: kernel-janitors
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;
}
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [xfs-masters] [PATCH] remove BPCSHIFT and NB?P? macros - was
2007-11-16 5:34 ` [PATCH] remove BPCSHIFT and NB?P? macros - was fs/xfs: remove duplicated Timothy Shimmin
@ 2007-11-16 6:15 ` Lachlan McIlroy
0 siblings, 0 replies; 17+ messages in thread
From: Lachlan McIlroy @ 2007-11-16 6:15 UTC (permalink / raw)
To: xfs-masters; +Cc: Eric Sandeen, xfs, kernel-janitors
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;
> }
>
>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [xfs-masters] [PATCH] remove BPCSHIFT and NB?P? macros - was fs/xfs: remove duplicated defines
@ 2007-11-16 6:15 ` Lachlan McIlroy
0 siblings, 0 replies; 17+ messages in thread
From: Lachlan McIlroy @ 2007-11-16 6:15 UTC (permalink / raw)
To: xfs-masters; +Cc: Eric Sandeen, xfs, kernel-janitors
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;
> }
>
>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread