Re: [reiserfs-dev] Re: [PATCH] sparc32: wrong type of nlink_t

From: Hans Reiser (reiser@namesys.com)
Date: Wed Sep 04 2002 - 15:31:36 EST


Dave Kleikamp wrote:

>>> Against 2.4.20-pre5 - fix up the type of nlink_t. This makes jfs and
>>> reiserfs stop complaining about comparisons always turning up false
>>> due to limited range of data type.
>>>
>>>If you change this, you change the types exported to userspace
>>>which will break everything.
>>>
>>>
>>Right. Here's a corresponding reiserfs/jfs fix, then. I've checked the
>>constants aren't used for anything else except nlink overflow alerts.
>>
>>
>
>I don't like this fix. I know 32767 is a lot of links, but I don't like
>artificially lowering a limit like this just because one architecture
>defines nlink_t incorrectly. I'd rather get rid of the compiler warnings
>with a cast in the few places the limit is checked, even though that is
>a little bit ugly.
>
>
>
>>diff -urN linux-2.4.20-pre5/fs/jfs/jfs_filsys.h linux-2.4.20-pre5.n/fs/jfs/jfs_filsys.h
>>--- linux-2.4.20-pre5/fs/jfs/jfs_filsys.h 2002-09-01 11:31:44.000000000 +0200
>>+++ linux-2.4.20-pre5.n/fs/jfs/jfs_filsys.h 2002-09-01 11:30:13.000000000 +0200
>>@@ -125,7 +125,8 @@
>> #define MAXBLOCKSIZE 4096
>> #define MAXFILESIZE ((s64)1 << 52)
>>
>>-#define JFS_LINK_MAX 65535 /* nlink_t is unsigned short */
>>+/* the shortest nlink_t there is is sparc's signed short */
>>+#define JFS_LINK_MAX 32767
>>
>> /* Minimum number of bytes supported for a JFS partition */
>> #define MINJFS (0x1000000)
>>diff -urN linux-2.4.20-pre5/include/linux/reiserfs_fs.h linux-2.4.20-pre5.n/include/linux/reiserfs_fs.h
>>--- linux-2.4.20-pre5/include/linux/reiserfs_fs.h 2002-09-01 11:31:45.000000000 +0200
>>+++ linux-2.4.20-pre5.n/include/linux/reiserfs_fs.h 2002-09-01 11:23:30.000000000 +0200
>>@@ -1185,10 +1185,12 @@
>> #define MAX_B_NUM MAX_UL_INT
>> #define MAX_FC_NUM MAX_US_INT
>>
>>-
>>-/* the purpose is to detect overflow of an unsigned short */
>>-#define REISERFS_LINK_MAX (MAX_US_INT - 1000)
>>-
>>+/* the original purpose was to detect a possible overflow
>>+ * of an unsigned short nlink_t. However, there are archs
>>+ * that only provide a signed short nlink_t, so this will
>>+ * have to start ringing a wee bit earlier.
>>+ */
>>+#define REISERFS_LINK_MAX (0x7fff - 1000)
>>
>> /* The following defines are used in reiserfs_insert_item and reiserfs_append_item */
>> #define REISERFS_KERNEL_MEM 0 /* reiserfs kernel memory mode */
>>
>>
>
>Thanks,
>Shaggy
>
>
>
I think you are sort of right. You are right to dislike this patch for
the reasons you state. The proper fix should be to make the result of
the limit computation be accurately architecture specific. I wasn't
paying enough attention --- hardcoding

0x7fff - 1000

is ugly. We need to find some appropriate #define to subtract 1000 from.

Green, please scan the code for the magic constant to compute from, and
code something unless Dave or someone does it before you.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sat Sep 07 2002 - 22:00:22 EST