2002-09-01 08:51:24

by Tomas Szepe

[permalink] [raw]
Subject: [PATCH] sparc32: wrong type of nlink_t

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.

T.

--- linux-2.4.19/include/asm-sparc/posix_types.h 2000-01-16 07:08:29.000000000 +0100
+++ linux-2.4.20-pre5/include/asm-sparc/posix_types.h 2002-09-01 10:41:35.000000000 +0200
@@ -21,7 +21,7 @@
typedef unsigned long __kernel_ino_t;
typedef unsigned short __kernel_mode_t;
typedef unsigned short __kernel_umode_t;
-typedef short __kernel_nlink_t;
+typedef unsigned short __kernel_nlink_t;
typedef long __kernel_daddr_t;
typedef long __kernel_off_t;
typedef char * __kernel_caddr_t;


2002-09-01 08:55:48

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] sparc32: wrong type of nlink_t

From: Tomas Szepe <[email protected]>
Date: Sun, 1 Sep 2002 10:55:24 +0200

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.

2002-09-01 09:40:08

by Tomas Szepe

[permalink] [raw]
Subject: Re: [PATCH] sparc32: wrong type of nlink_t

> From: Tomas Szepe <[email protected]>
> Date: Sun, 1 Sep 2002 10:55:24 +0200
>
> 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.

T.


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 */

2002-09-04 20:15:55

by Dave Kleikamp

[permalink] [raw]
Subject: Re: [PATCH] sparc32: wrong type of nlink_t

> > 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

--
David Kleikamp
IBM Linux Technology Center

2002-09-04 20:23:09

by Chris Mason

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

On Wed, 2002-09-04 at 16:18, 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.
>

The patch will probably cause reiserfs problems as well, we've already
got people with > 32767 links on disk, going to a lower number will
confuse things.

-chris


2002-09-04 23:36:11

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] sparc32: wrong type of nlink_t

From: Dave Kleikamp <[email protected]>
Date: Wed, 4 Sep 2002 15:18:05 -0500 (CDT)

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.

"incorrectly"? There are no correct or incorrect values for any
of these types, which is why they are defined on a per-platform
basis.

If you impose different limits on different platforms, that means
that a jfs/reiserfs filesystem that works properly on one platform
may not function properly on another.

That is something I'd certainly deem "incorrect" :-)

Every other filesystem can be plugged into an arbitrary Linux platform
and be expected to work properly, don't make jfs/reiserfs an exception
to this.

2002-09-04 23:36:44

by David Miller

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

From: Chris Mason <[email protected]>
Date: 04 Sep 2002 16:29:21 -0400

The patch will probably cause reiserfs problems as well, we've already
got people with > 32767 links on disk, going to a lower number will
confuse things.

And that means you already have reiserfs partitions that cannot
be used on other Linux platforms. That's pretty bad.


2002-09-06 08:48:14

by David Woodhouse

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


[email protected] said:
> > The patch will probably cause reiserfs problems as well, we've
> > already got people with > 32767 links on disk, going to a lower
> > number will confuse things.
> And that means you already have reiserfs partitions that cannot be
> used on other Linux platforms. That's pretty bad.

Surely a file system with > 32Ki links can be _used_ on sparc, you just
can't return a correct value in st_nlink. For directories, you could
perhaps set st_nlink to '1', which many things will interpret as
"don't know". For files, I'm not sure -- but even just setting it to
min(32767, real_nlink) would suffice, surely? It's inaccurate but it's
better than the idea that the file system just cannot be mounted.

Is there a requirement to stop allowing hard links (or subdirectories)
to be made when nlink reaches the maximum representable to user space?
Obviously you have to do it if you're keeping an nlink count on the
medium and you'd overflow _that_, but should we return -EMLINK even if we
could represent the new hard link on the file system?

--
dwmw2