All of lore.kernel.org
 help / color / mirror / Atom feed
* IA64 ino_t incorrectly sized?
@ 2003-09-17  7:10 Nathan Scott
  2003-09-17 14:33 ` Jes Sorensen
                   ` (32 more replies)
  0 siblings, 33 replies; 34+ messages in thread
From: Nathan Scott @ 2003-09-17  7:10 UTC (permalink / raw)
  To: linux-ia64

Hi there,

I've been doing some large filesystem testing on XFS, and noticed
that several of our regression tests are failing.  One particular
problem area is xfsdump and its use of the getdents64 interface.
I believe I've traced the problem back to the definition of ino_t
(aka __kernel_ino_t) as an unsigned int in the platform-specific
headers, and I suspect this should instead be an unsigned long on
IA64.  Here's the rationale:

- all of IA64 userspace (glibc) allows for 64 bit inode numbers;
  see /usr/include/bits/types.h in particular, and __ino_t which
  is used in the stat and getdents64 syscalls.
- when we come into the kernel via sys_getdents64, we call into
  filesystem specific code via fs/readdir.c::vfs_readdir and the
  readdir file operation.
- this passes a filldir_t routine into the fs, which the fs uses
  to fill in the getdents buffer for each directory entry.
- filldir64 from sys_getdents64 makes use of the linux_dirent64
  structure internally (with a u64 d_ino) and is passed an ino_t
  as the inode number.
- so, when we fill in each directory entry we're silently chopping
  off the high 32 bits of the inode number (casting from the XFS
  64 bit inode to the ino_t argument of the filldir routines),
  which causes problems for xfsdump as it is looking at the inode
  numbers in the getdents buffer returned from the kernel.
- I can see no reason to have this restriction on IA64, because
  userspace does allow for 64 bit inode numbers.

Does anyone know why the IA64 platform-specific ino_t definition
is an int and not a long?  Patch below fixes this problem for me
but I wonder if there will be side-effects I haven't considered
(i.e. was there a reason for making this 32 bits originally?).
If not, could the IA64 maintainers push this patch around to the
official kernel trees for me?  (pretty please)

many thanks.

-- 
Nathan


--- /usr/tmp/TmpDir.16879-0/linux/include/asm-ia64/posix_types.h_1.1	Wed Sep 17 16:04:09 2003
+++ linux/include/asm-ia64/posix_types.h	Wed Sep 17 11:29:30 2003
@@ -11,7 +11,7 @@
  */
 
 typedef unsigned int	__kernel_dev_t;
-typedef unsigned int	__kernel_ino_t;
+typedef unsigned long	__kernel_ino_t;
 typedef unsigned int	__kernel_mode_t;
 typedef unsigned int	__kernel_nlink_t;
 typedef long		__kernel_off_t;

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
@ 2003-09-17 14:33 ` Jes Sorensen
  2003-09-17 17:26 ` David Mosberger
                   ` (31 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Jes Sorensen @ 2003-09-17 14:33 UTC (permalink / raw)
  To: linux-ia64

>>>>> "Nathan" = Nathan Scott <nathans@sgi.com> writes:

Nathan> Does anyone know why the IA64 platform-specific ino_t
Nathan> definition is an int and not a long?  Patch below fixes this
Nathan> problem for me but I wonder if there will be side-effects I
Nathan> haven't considered (i.e. was there a reason for making this 32
Nathan> bits originally?).  If not, could the IA64 maintainers push
Nathan> this patch around to the official kernel trees for me?
Nathan> (pretty please)

Hi Nathan,

I am actually surprised it's still a 32 bit int in the kernel. I
deliberately used 64 bit types in glibc so it could be done
right. Must have slipped on fixing the kernel for this one.

David?

Cheers,
Jes

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
  2003-09-17 14:33 ` Jes Sorensen
@ 2003-09-17 17:26 ` David Mosberger
  2003-09-29  5:52 ` Nathan Scott
                   ` (30 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: David Mosberger @ 2003-09-17 17:26 UTC (permalink / raw)
  To: linux-ia64

>>>>> On 17 Sep 2003 10:33:47 -0400, Jes Sorensen <jes@wildopensource.com> said:

>>>>> "Nathan" = Nathan Scott <nathans@sgi.com> writes:
  Nathan> Does anyone know why the IA64 platform-specific ino_t
  Nathan> definition is an int and not a long?  Patch below fixes this
  Nathan> problem for me but I wonder if there will be side-effects I
  Nathan> haven't considered (i.e. was there a reason for making this
  Nathan> 32 bits originally?).  If not, could the IA64 maintainers
  Nathan> push this patch around to the official kernel trees for me?
  Nathan> (pretty please)

  Jes> Hi Nathan,

  Jes> I am actually surprised it's still a 32 bit int in the
  Jes> kernel. I deliberately used 64 bit types in glibc so it could
  Jes> be done right. Must have slipped on fixing the kernel for this
  Jes> one.

  Jes> David?

Extending ino_t to 64 bits came up last October [1].  AFAIK, nobody
bothered to investigate & send a patch, so things didn't change since
then.

	--david

[1] http://www.gelato.unsw.edu.au/linux-ia64/0210/3952.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
  2003-09-17 14:33 ` Jes Sorensen
  2003-09-17 17:26 ` David Mosberger
@ 2003-09-29  5:52 ` Nathan Scott
  2003-10-08 23:51 ` David Mosberger
                   ` (29 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Nathan Scott @ 2003-09-29  5:52 UTC (permalink / raw)
  To: linux-ia64

On Wed, Sep 17, 2003 at 10:26:25AM -0700, David Mosberger wrote:
> >>>>> On 17 Sep 2003 10:33:47 -0400, Jes Sorensen <jes@wildopensource.com> said:
> 
> >>>>> "Nathan" = Nathan Scott <nathans@sgi.com> writes:
>   Nathan> Does anyone know why the IA64 platform-specific ino_t
>   Nathan> definition is an int and not a long?  Patch below fixes this
>   Nathan> problem for me but I wonder if there will be side-effects I
>   Nathan> haven't considered (i.e. was there a reason for making this
>   Nathan> 32 bits originally?).  If not, could the IA64 maintainers
>   Nathan> push this patch around to the official kernel trees for me?
>   Nathan> (pretty please)
> 
>   Jes> Hi Nathan,
> 
>   Jes> I am actually surprised it's still a 32 bit int in the
>   Jes> kernel. I deliberately used 64 bit types in glibc so it could
>   Jes> be done right. Must have slipped on fixing the kernel for this
>   Jes> one.
> 
>   Jes> David?
> 
> Extending ino_t to 64 bits came up last October [1].  AFAIK, nobody
> bothered to investigate & send a patch, so things didn't change since
> then.
> 
> 	--david
> 
> [1] http://www.gelato.unsw.edu.au/linux-ia64/0210/3952.html
> 

I notice a big batch of IA64 changes has just gone into 2.6-test6,
but this change seems to be missing.  Is it in someones queue for
next time or do I need to describe the problem more clearly?

The investigation and the patch I sent are available here[2].

cheers.

-- 
Nathan

[2] http://www.gelato.unsw.edu.au/linux-ia64/0309/6681.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (2 preceding siblings ...)
  2003-09-29  5:52 ` Nathan Scott
@ 2003-10-08 23:51 ` David Mosberger
  2003-10-09  1:25 ` Nathan Scott
                   ` (28 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: David Mosberger @ 2003-10-08 23:51 UTC (permalink / raw)
  To: linux-ia64

>>>>> On Mon, 29 Sep 2003 15:52:56 +1000, Nathan Scott <nathans@sgi.com> said:

  >>  Extending ino_t to 64 bits came up last October [1].  AFAIK,
  >> nobody bothered to investigate & send a patch, so things didn't
  >> change since then.

  >> --david

  >> [1] http://www.gelato.unsw.edu.au/linux-ia64/0210/3952.html


  Nathan> I notice a big batch of IA64 changes has just gone into
  Nathan> 2.6-test6, but this change seems to be missing.  Is it in
  Nathan> someones queue for next time or do I need to describe the
  Nathan> problem more clearly?

  Nathan> The investigation and the patch I sent are available
  Nathan> here[2].

  Nathan> cheers.

  Nathan> -- Nathan

  Nathan> [2] http://www.gelato.unsw.edu.au/linux-ia64/0309/6681.html

The mail you're referring to talks about getdents64() only.  What I
didn't see is an argument why this is the _only_ user visible
interface that might be affected.  Are there any other interfaces
(syscalls, /proc/whatever, etc.) that may directly or indirectly be
affected?  If not and if the change has been run through a reasonable
test-suite (LTP?) without ill effects, I'm certainly OK with the
change.

	--david

PS: It would help to get this resolved quickly, e.g., before test8.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (3 preceding siblings ...)
  2003-10-08 23:51 ` David Mosberger
@ 2003-10-09  1:25 ` Nathan Scott
  2003-10-09  1:57 ` David Mosberger
                   ` (27 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Nathan Scott @ 2003-10-09  1:25 UTC (permalink / raw)
  To: linux-ia64

hi David,

On Wed, Oct 08, 2003 at 04:51:09PM -0700, David Mosberger wrote:
> 
> The mail you're referring to talks about getdents64() only.  What I
> didn't see is an argument why this is the _only_ user visible
> interface that might be affected.  Are there any other interfaces
> (syscalls, /proc/whatever, etc.) that may directly or indirectly be
> affected?

The problem is not the interfaces - they tend to use "long"
directly and will be unaffected.  When I started running our
tests with large inode numbers, for the most part everything
just worked.  Thats because, as Jes said, the IA64 userspace
(libc, etc) is already 64 bit clean, and inode numbers are a
long (directly, not via ino_t) in much of the kernel.

The problem is the ia64 kernel sometimes chops off the high 32
bits of an inode number for no good reason.  Note that ino_t
is not the kernels ubiquitous inode number representation, its
only used for a subset of the internal interfaces and not for
kernel/user interfaces directly.

e.g. the ia64 include/asm-ia64/stat.h uses long for the inode
number field, and the struct inode i_ino is a long also.  So,
for the most part everything works as is - there are corner
cases (i.e. getdents) that this fixes though, as I explained.
It does not change any external interfaces (if it did they'd
be broken already, but I haven't seen any evidence of that in
my testing).

> If not and if the change has been run through a reasonable
> test-suite (LTP?) without ill effects, I'm certainly OK with the
> change.

With this fix, the XFS test suite passes on filesystems with
all inode numbers forced into the >2^32 range.  This includes
a large amount of filesystem/vfs stress covering all fs ops.

Without the fix, getdents and stat can report different inode
numbers for the same file, even though both structures have
sufficient bits for the correct inode number.

cheers.

ps: all of the above holds for 2.4 as well, the same patch
will apply cleanly there.

-- 
Nathan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (4 preceding siblings ...)
  2003-10-09  1:25 ` Nathan Scott
@ 2003-10-09  1:57 ` David Mosberger
  2003-10-09  3:15 ` Nathan Scott
                   ` (26 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: David Mosberger @ 2003-10-09  1:57 UTC (permalink / raw)
  To: linux-ia64

>>>>> On Thu, 9 Oct 2003 11:25:58 +1000, Nathan Scott <nathans@sgi.com> said:

  Nathan> The problem is not the interfaces - they tend to use "long"
  Nathan> directly and will be unaffected.  When I started running our
  Nathan> tests with large inode numbers, for the most part everything
  Nathan> just worked.

Argh, of course it's about interfaces.  We don't want to break
something unknowingly because of this change.  Saying that "for the
most part everything just worked" isn't exactly reassuring!

  Nathan> e.g. the ia64 include/asm-ia64/stat.h uses long for the
  Nathan> inode number field, and the struct inode i_ino is a long
  Nathan> also.  So, for the most part everything works as is - there
  Nathan> are corner cases (i.e. getdents) that this fixes though, as
  Nathan> I explained.  It does not change any external interfaces (if
  Nathan> it did they'd be broken already, but I haven't seen any
  Nathan> evidence of that in my testing).

Are you saying that you reviewed the entire syscall interface and did
a reasonable review of anything else (e.g., /proc) that might expose
ino_t in one way or another?

	--david

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (5 preceding siblings ...)
  2003-10-09  1:57 ` David Mosberger
@ 2003-10-09  3:15 ` Nathan Scott
  2003-10-09  3:53 ` David Mosberger
                   ` (25 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Nathan Scott @ 2003-10-09  3:15 UTC (permalink / raw)
  To: linux-ia64

On Wed, Oct 08, 2003 at 06:57:13PM -0700, David Mosberger wrote:
> >>>>> On Thu, 9 Oct 2003 11:25:58 +1000, Nathan Scott <nathans@sgi.com> said:
> 
>   Nathan> The problem is not the interfaces - they tend to use "long"
>   Nathan> directly and will be unaffected.  When I started running our
>   Nathan> tests with large inode numbers, for the most part everything
>   Nathan> just worked.
> 
> Argh, of course it's about interfaces.  We don't want to break
> something unknowingly because of this change.

Yes, I understand that.

> Saying that "for the
> most part everything just worked" isn't exactly reassuring!

You have misunderstood and/or I have not been clear enough.

"When I started running tests"... i.e. before I fixed this issue
in asm-ia64/posix_types.h ... "for the most part everything just
worked"... i.e. most stuff worked before, but not the thing I
fixed; but now it is fixed, and all our tests pass now, and I
know of no remaining issues.

I did not review every possible interface for use of ino_t, no.
I suppose that needs to be done, I don't have that kind of time
available just now.

cheers.

-- 
Nathan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (6 preceding siblings ...)
  2003-10-09  3:15 ` Nathan Scott
@ 2003-10-09  3:53 ` David Mosberger
  2003-10-09  4:55 ` Nathan Scott
                   ` (24 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: David Mosberger @ 2003-10-09  3:53 UTC (permalink / raw)
  To: linux-ia64

>>>>> On Thu, 9 Oct 2003 13:15:01 +1000, Nathan Scott <nathans@sgi.com> said:

  Nathan> I did not review every possible interface for use of ino_t,
  Nathan> no.  I suppose that needs to be done, I don't have that kind
  Nathan> of time available just now.

Well, then don't be surprised if it doesn't get fixed!

	--david

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (7 preceding siblings ...)
  2003-10-09  3:53 ` David Mosberger
@ 2003-10-09  4:55 ` Nathan Scott
  2003-10-09 20:46 ` David Mosberger
                   ` (23 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Nathan Scott @ 2003-10-09  4:55 UTC (permalink / raw)
  To: linux-ia64

On Wed, Oct 08, 2003 at 08:53:37PM -0700, David Mosberger wrote:
> >>>>> On Thu, 9 Oct 2003 13:15:01 +1000, Nathan Scott <nathans@sgi.com> said:
> 
>   Nathan> I did not review every possible interface for use of ino_t,
>   Nathan> no.  I suppose that needs to be done, I don't have that kind
>   Nathan> of time available just now.
> 
> Well, then don't be surprised if it doesn't get fixed!

s/fixed/merged/

I'm not surprised, and I can understand your position.  I'll
come back to do an audit when I can, if noone beats me to it
in the meantime.

cheers.

-- 
Nathan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (8 preceding siblings ...)
  2003-10-09  4:55 ` Nathan Scott
@ 2003-10-09 20:46 ` David Mosberger
  2003-10-10  2:22 ` Nathan Scott
                   ` (22 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: David Mosberger @ 2003-10-09 20:46 UTC (permalink / raw)
  To: linux-ia64

>>>>> On Thu, 9 Oct 2003 14:55:29 +1000, Nathan Scott <nathans@sgi.com> said:

  Nathan> On Wed, Oct 08, 2003 at 08:53:37PM -0700, David Mosberger wrote:
  >> >>>>> On Thu, 9 Oct 2003 13:15:01 +1000, Nathan Scott <nathans@sgi.com> said:

  Nathan> I did not review every possible interface for use of ino_t,
  Nathan> no.  I suppose that needs to be done, I don't have that kind
  Nathan> of time available just now.

  >> Well, then don't be surprised if it doesn't get fixed!

  Nathan> s/fixed/merged/

  Nathan> I'm not surprised, and I can understand your position.  I'll
  Nathan> come back to do an audit when I can, if noone beats me to it
  Nathan> in the meantime.

I doubt anyone will beat you to it.  I don't think the audit should
take a huge amount of time.  Perhaps half to a full day, just to make
sure we're not missing something subtle (such as ioctl()s or something
like that).

Also, note that the time window of making this change for 2.6 is
closing quickly.  Once 2.6.0 is out, it would be 2.7 material.

	--david

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (9 preceding siblings ...)
  2003-10-09 20:46 ` David Mosberger
@ 2003-10-10  2:22 ` Nathan Scott
  2003-10-15  1:25 ` Nathan Scott
                   ` (21 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Nathan Scott @ 2003-10-10  2:22 UTC (permalink / raw)
  To: linux-ia64

On Thu, Oct 09, 2003 at 01:46:56PM -0700, David Mosberger wrote:
>   Nathan> I'm not surprised, and I can understand your position.  I'll
>   Nathan> come back to do an audit when I can, if noone beats me to it
>   Nathan> in the meantime.
> 
> I doubt anyone will beat you to it.  I don't think the audit should
> take a huge amount of time.  Perhaps half to a full day, just to make
> sure we're not missing something subtle (such as ioctl()s or something
> like that).

I've started the audit, but I'm not finished yet & gotta run
(short vacation, sorry wife says no laptop allowed).  I think
I have found two possible issues, I will send out an updated
patch next week.

cheers.

-- 
Nathan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (10 preceding siblings ...)
  2003-10-10  2:22 ` Nathan Scott
@ 2003-10-15  1:25 ` Nathan Scott
  2003-10-15  1:48 ` Andrew Morton
                   ` (20 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Nathan Scott @ 2003-10-15  1:25 UTC (permalink / raw)
  To: linux-ia64

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

hi David,

[ Below are updated patches.  I've added Christoph and Andrew
onto the discussion because the patches add __ia64__ ifdef's
into core kernel headers, and they've shown me cleaner ways
to do this sort of thing in the past. ]

On Wed, Oct 08, 2003 at 06:57:13PM -0700, David Mosberger wrote:
> >>>>> On Thu, 9 Oct 2003 11:25:58 +1000, Nathan Scott <nathans@sgi.com> said:
>   Nathan> e.g. the ia64 include/asm-ia64/stat.h uses long for the
>   Nathan> inode number field, and the struct inode i_ino is a long
>   Nathan> also.  So, for the most part everything works as is - there
>   Nathan> are corner cases (i.e. getdents) that this fixes though, as
>   Nathan> I explained.  It does not change any external interfaces (if
>   Nathan> it did they'd be broken already, but I haven't seen any
>   Nathan> evidence of that in my testing).
> 
> Are you saying that you reviewed the entire syscall interface and did
> a reasonable review of anything else (e.g., /proc) that might expose
> ino_t in one way or another?

So, my brave assertion that no code would be exporting a type
called "__kernel_ino_t" turns out to be incorrect (/me hangs
head in shame) as there are two cases I see where this type
is exported to userspace.

Here's some more details about what I've reviewed and found.
If I've overlooked any areas let me know.

system call interface -- I examined the 2.4 IA64 system call
table and each of the structures passed across it in detail.
This revealed that the ustat and NFS system calls pass around
binary structures with __kernel_ino_t fields (see my updated
patches).  I then diff'd the 2.4 and 2.6 asm-ia64/unistd.h
and reviewed each of the new syscalls - there are no new 2.6
interfaces that deal with an ino_t.

procfs interfaces -- the one procfs interface I found dealing
with inode numbers is the /proc/PID/maps file.  In 2.4, this is
done in fs/proc/array.c::proc_pid_maps_get_line and in 2.6 its
done in fs/proc/task_mmu.c::show_map.  In both places, the ino
is copied out of (struct inode)->i_ino into a local unsigned
long and then formatted via "%lu".  Hence, ino_t doesn't come
into the picture here and we are not exposed to __kernel_ino_t
changes in either kernel.

ioctl interfaces -- there are several places in arch/ia64/ia32/
ia32_ioctl.c and sys_ia32.c where we copy an ino_t into an
unsigned int...
	sys_ia32.c::filldir32
	sys_ia32.c::fillonedir32
	ia32_ioctl.c::put_dirent32

These would change from copying 'unsigned int'->'unsigned int'
into 'unsigned long'->'unsigned int'.  It turns out that this
is already being done in other places, so I think its OK; e.g.
sys_ia32.c::putstat copies a kbuf->st_ino (unsigned long, from
the IA64 version of struct stat) into an ubuf->st_ino (uint).

From reviewing the code with Keith Owens, it looks like this
copies the first 4 bytes (sizeof the specified field in the
user buffer) and we've "got lucky" here due to our endianness
(otherwise we would get all zeroes).  Not sure if this was
by luck or by design, but obviously it works as is.


Anyway, here's the two patches (2.6 & 2.4).  The 2.4 version
no longer applies cleanly to 2.6 due to other kernel changes,
and the 2.6 kernel deprecates one of the 2.4 NFS structures.

I'd appreciate any suggestions about a cleaner way to do the
common types.h and nfsd/syscall.h header changes.

cheers.

-- 
Nathan

[-- Attachment #2: ino_t-2.6.patch --]
[-- Type: text/plain, Size: 1813 bytes --]


===========================================================================
linux/include/asm-ia64/posix_types.h
===========================================================================

--- /usr/tmp/TmpDir.2032422-0/linux/include/asm-ia64/posix_types.h_1.4	Wed Oct 15 07:57:15 2003
+++ linux/include/asm-ia64/posix_types.h	Tue Oct 14 16:23:17 2003
@@ -10,7 +10,7 @@
  *	David Mosberger-Tang <davidm@hpl.hp.com>
  */
 
-typedef unsigned int	__kernel_ino_t;
+typedef unsigned long	__kernel_ino_t;
 typedef unsigned int	__kernel_mode_t;
 typedef unsigned int	__kernel_nlink_t;
 typedef long		__kernel_off_t;

===========================================================================
linux/include/linux/nfsd/syscall.h
===========================================================================

--- /usr/tmp/TmpDir.2032422-0/linux/include/linux/nfsd/syscall.h_1.11	Wed Oct 15 07:57:15 2003
+++ linux/include/linux/nfsd/syscall.h	Tue Oct 14 16:18:07 2003
@@ -60,7 +60,11 @@
 	char			ex_client[NFSCLNT_IDMAX+1];
 	char			ex_path[NFS_MAXPATHLEN+1];
 	__kernel_old_dev_t	ex_dev;
+#ifdef __ia64__
+	unsigned int		ex_ino;	/* ABI preservation */
+#else
 	__kernel_ino_t		ex_ino;
+#endif
 	int			ex_flags;
 	__kernel_uid_t		ex_anon_uid;
 	__kernel_gid_t		ex_anon_gid;

===========================================================================
linux/include/linux/types.h
===========================================================================

--- /usr/tmp/TmpDir.2032422-0/linux/include/linux/types.h_1.14	Wed Oct 15 07:57:15 2003
+++ linux/include/linux/types.h	Tue Oct 14 16:09:06 2003
@@ -150,7 +150,11 @@
 
 struct ustat {
 	__kernel_daddr_t	f_tfree;
+#ifdef __ia64__
+	unsigned int		f_tinode;	/* ABI preservation */
+#else
 	__kernel_ino_t		f_tinode;
+#endif
 	char			f_fname[6];
 	char			f_fpack[6];
 };

[-- Attachment #3: ino_t-2.4.patch --]
[-- Type: text/plain, Size: 2023 bytes --]


===========================================================================
linux/include/asm-ia64/posix_types.h
===========================================================================

--- /usr/tmp/TmpDir.14060-0/linux/include/asm-ia64/posix_types.h_1.1	Wed Oct 15 09:13:59 2003
+++ linux/include/asm-ia64/posix_types.h	Fri Oct 10 11:48:09 2003
@@ -11,7 +11,7 @@
  */
 
 typedef unsigned int	__kernel_dev_t;
-typedef unsigned int	__kernel_ino_t;
+typedef unsigned long	__kernel_ino_t;
 typedef unsigned int	__kernel_mode_t;
 typedef unsigned int	__kernel_nlink_t;
 typedef long		__kernel_off_t;

===========================================================================
linux/include/linux/nfsd/syscall.h
===========================================================================

--- /usr/tmp/TmpDir.14060-0/linux/include/linux/nfsd/syscall.h_1.6	Wed Oct 15 09:13:59 2003
+++ linux/include/linux/nfsd/syscall.h	Tue Oct 14 16:06:11 2003
@@ -60,7 +60,11 @@
 	char			ex_client[NFSCLNT_IDMAX+1];
 	char			ex_path[NFS_MAXPATHLEN+1];
 	__kernel_dev_t		ex_dev;
+#ifdef __ia64__
+	unsigned int		ex_ino;	/* ABI preservation */
+#else
 	__kernel_ino_t		ex_ino;
+#endif
 	int			ex_flags;
 	__kernel_uid_t		ex_anon_uid;
 	__kernel_gid_t		ex_anon_gid;
@@ -81,7 +85,11 @@
 struct nfsctl_fhparm {
 	struct sockaddr		gf_addr;
 	__kernel_dev_t		gf_dev;
+#ifdef __ia64__
+	unsigned int		gf_ino;	/* ABI preservation */
+#else
 	__kernel_ino_t		gf_ino;
+#endif
 	int			gf_version;
 };
 

===========================================================================
linux/include/linux/types.h
===========================================================================

--- /usr/tmp/TmpDir.14060-0/linux/include/linux/types.h_1.9	Wed Oct 15 09:13:59 2003
+++ linux/include/linux/types.h	Tue Oct 14 16:05:36 2003
@@ -129,7 +129,11 @@
 
 struct ustat {
 	__kernel_daddr_t	f_tfree;
+#ifdef __ia64__
+	unsigned int		f_tinode;	/* ABI preservation */
+#else
 	__kernel_ino_t		f_tinode;
+#endif
 	char			f_fname[6];
 	char			f_fpack[6];
 };

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (11 preceding siblings ...)
  2003-10-15  1:25 ` Nathan Scott
@ 2003-10-15  1:48 ` Andrew Morton
  2003-10-15  4:47 ` David Mosberger
                   ` (19 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Andrew Morton @ 2003-10-15  1:48 UTC (permalink / raw)
  To: linux-ia64

Nathan Scott <nathans@sgi.com> wrote:
>
>  Anyway, here's the two patches (2.6 & 2.4).  The 2.4 version
>  no longer applies cleanly to 2.6 due to other kernel changes,
>  and the 2.6 kernel deprecates one of the 2.4 NFS structures.
> 
>  I'd appreciate any suggestions about a cleaner way to do the
>  common types.h and nfsd/syscall.h header changes.

Can't think of anything clever really.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (12 preceding siblings ...)
  2003-10-15  1:48 ` Andrew Morton
@ 2003-10-15  4:47 ` David Mosberger
  2003-10-15  5:18 ` Andrew Morton
                   ` (18 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: David Mosberger @ 2003-10-15  4:47 UTC (permalink / raw)
  To: linux-ia64

Hi Nathan,

Thanks for doing the thorough analysis!

>>>>> On Wed, 15 Oct 2003 11:25:04 +1000, Nathan Scott <nathans@sgi.com> said:

  Nathan> system call interface -- I examined the 2.4 IA64 system call
  Nathan> table and each of the structures passed across it in detail.
  Nathan> This revealed that the ustat and NFS system calls pass around
  Nathan> binary structures with __kernel_ino_t fields (see my updated
  Nathan> patches).  I then diff'd the 2.4 and 2.6 asm-ia64/unistd.h
  Nathan> and reviewed each of the new syscalls - there are no new 2.6
  Nathan> interfaces that deal with an ino_t.

Those are nasty.  I suppose your patch works, but wouldn't it mean
that NFS-export and/or ustat() of XFS file systems would fail?

ustat() we could handle pretty easily, by introducing a new syscall
number which uses the "long" ino_t.  Then we just write a small
wrapper for the old ustat() syscall.

NFS is nastier (ugly multiplexed syscalls...).  I suppose it could be
handled by incrementing NFSCTL_VERSION, but I'm not sure of all the
implications of doing that (probably not what we want).  Actually,
I think we could handle NFS the same way as ustat(): change the
ino_t to long and then have a wrapper for the old nfsctl() syscall
which passes through everything except for some nfsctl_export munging.

Do you want to try this?  If you do, you could use syscall numbers
1259 and 1260.  I don't expect/hope that other syscalls will be added
this late in the game.

	--david

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (13 preceding siblings ...)
  2003-10-15  4:47 ` David Mosberger
@ 2003-10-15  5:18 ` Andrew Morton
  2003-10-15  6:06 ` Nathan Scott
                   ` (17 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Andrew Morton @ 2003-10-15  5:18 UTC (permalink / raw)
  To: linux-ia64

David Mosberger <davidm@napali.hpl.hp.com> wrote:
>
> NFS is nastier (ugly multiplexed syscalls...). 

Well I think that whole interface is deprecated anyway and the new nfsutils
doesn't use it - it uses exportfs instead.

You'd need to check with Neil, but if I'm right you could just say "use
current nfsutils on ia64".



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (14 preceding siblings ...)
  2003-10-15  5:18 ` Andrew Morton
@ 2003-10-15  6:06 ` Nathan Scott
  2003-10-15  6:16 ` Nathan Scott
                   ` (16 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Nathan Scott @ 2003-10-15  6:06 UTC (permalink / raw)
  To: linux-ia64

On Tue, Oct 14, 2003 at 09:47:41PM -0700, David Mosberger wrote:
> Hi Nathan,
> 
> Thanks for doing the thorough analysis!

No problem.  Thanks for the prodding. ;-)

> >>>>> On Wed, 15 Oct 2003 11:25:04 +1000, Nathan Scott <nathans@sgi.com> said:
> 
>   Nathan> system call interface -- I examined the 2.4 IA64 system call
>   Nathan> table and each of the structures passed across it in detail.
>   Nathan> This revealed that the ustat and NFS system calls pass around
>   Nathan> binary structures with __kernel_ino_t fields (see my updated
>   Nathan> patches).  I then diff'd the 2.4 and 2.6 asm-ia64/unistd.h
>   Nathan> and reviewed each of the new syscalls - there are no new 2.6
>   Nathan> interfaces that deal with an ino_t.
> 
> Those are nasty.  I suppose your patch works, but wouldn't it mean
> that NFS-export and/or ustat() of XFS file systems would fail?

It turns out that neither is a problem for us in practice.

In the case of ustat(2) ...
	ino_t     f_tinode;      /* Number of free inodes */
is meaningless on those filesystems (like XFS) which don't allocate a
fixed set of inodes at mkfs time.  It's only an "ino_t" for hysterical
raisins too (the count of free inodes? != an inode number!) - I notice
IRIX defines this in exactly the same way, I guess this came from SVR4
verbatim.  I'm hard pressed finding an application that uses this, and
I was quite surprised to find it in Linux at all.

For the NFS case - I asked one of the local NFS gurus to look over the
changes yesterday, and he tells me that field is only used to hold the
root inode number of a filesystem.  So, for XFS (and I'd imagine most
other filesystems too) this is never going to be a problem - the root
never has a large inode number in XFS because it's allocated at mkfs
time and always from the first allocation group (where inode numbers
are small).

> ...
> 1259 and 1260.  I don't expect/hope that other syscalls will be added
> this late in the game.

No, nor do I.  I don't think we should go that far though, certainly
it seems all our needs in XFS will be met without adding these calls.

cheers.

-- 
Nathan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (15 preceding siblings ...)
  2003-10-15  6:06 ` Nathan Scott
@ 2003-10-15  6:16 ` Nathan Scott
  2003-10-15  6:21 ` David Mosberger
                   ` (15 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Nathan Scott @ 2003-10-15  6:16 UTC (permalink / raw)
  To: linux-ia64

On Tue, Oct 14, 2003 at 06:48:07PM -0700, Andrew Morton wrote:
> Nathan Scott <nathans@sgi.com> wrote:
> >
> >  Anyway, here's the two patches (2.6 & 2.4).  The 2.4 version
> >  no longer applies cleanly to 2.6 due to other kernel changes,
> >  and the 2.6 kernel deprecates one of the 2.4 NFS structures.
> > 
> >  I'd appreciate any suggestions about a cleaner way to do the
> >  common types.h and nfsd/syscall.h header changes.
> 
> Can't think of anything clever really.
> 

One alternate approach would be to introduce __kernel_ustatino_t
and __kernel_nfsdino_t into every architecture's posix_types.h
(defining them as __kernel_ino_t everywhere but IA64, where they
would be unsigned int).  This would do away with any conditional
code in headers, but I'm not sure its the right approach at this
stage?

cheers.

-- 
Nathan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (16 preceding siblings ...)
  2003-10-15  6:16 ` Nathan Scott
@ 2003-10-15  6:21 ` David Mosberger
  2003-10-15  6:28 ` Andrew Morton
                   ` (14 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: David Mosberger @ 2003-10-15  6:21 UTC (permalink / raw)
  To: linux-ia64

>>>>> On Wed, 15 Oct 2003 16:06:02 +1000, Nathan Scott <nathans@sgi.com> said:

  Nathan> It turns out that neither is a problem for us in practice.

Sounds find to me, then.  Except, I'd replace #ifdef __ia64__ with
#ifdef CONFIG_IA64, so you're relying (less) on compiler magic.

	--david

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (17 preceding siblings ...)
  2003-10-15  6:21 ` David Mosberger
@ 2003-10-15  6:28 ` Andrew Morton
  2003-10-15  6:34 ` Nathan Scott
                   ` (13 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Andrew Morton @ 2003-10-15  6:28 UTC (permalink / raw)
  To: linux-ia64

Nathan Scott <nathans@sgi.com> wrote:
>
>  One alternate approach would be to introduce __kernel_ustatino_t
>  and __kernel_nfsdino_t into every architecture's posix_types.h
>  (defining them as __kernel_ino_t everywhere but IA64, where they
>  would be unsigned int).  This would do away with any conditional
>  code in headers, but I'm not sure its the right approach at this
>  stage?

Sounds overly elaborate to me.  The patch which you had looked liveable-with.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (18 preceding siblings ...)
  2003-10-15  6:28 ` Andrew Morton
@ 2003-10-15  6:34 ` Nathan Scott
  2003-10-15 12:42 ` Andi Kleen
                   ` (12 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Nathan Scott @ 2003-10-15  6:34 UTC (permalink / raw)
  To: linux-ia64

On Tue, Oct 14, 2003 at 11:21:09PM -0700, David Mosberger wrote:
> >>>>> On Wed, 15 Oct 2003 16:06:02 +1000, Nathan Scott <nathans@sgi.com> said:
> 
>   Nathan> It turns out that neither is a problem for us in practice.
> 
> Sounds find to me, then.  Except, I'd replace #ifdef __ia64__ with
> #ifdef CONFIG_IA64, so you're relying (less) on compiler magic.
> 

Oh.  I had avoided that because it requires any sources including
these headers to have already included linux/config.h, which they
may not be doing.  linux/types.h is included by userspace code too,
I believe, so may be an issue there too.

cheers.

-- 
Nathan

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (19 preceding siblings ...)
  2003-10-15  6:34 ` Nathan Scott
@ 2003-10-15 12:42 ` Andi Kleen
  2003-10-15 12:54 ` Christoph Hellwig
                   ` (11 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Andi Kleen @ 2003-10-15 12:42 UTC (permalink / raw)
  To: linux-ia64

>  	__kernel_old_dev_t	ex_dev;
> +#ifdef __ia64__
> +	unsigned int		ex_ino;	/* ABI preservation */
> +#else
>  	__kernel_ino_t		ex_ino;
> +#endif

Please don't make it #ifdef __ia64__, but some other define that other
architectures who may want to move the 64bit ino_t too can just set.

-Andi

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (20 preceding siblings ...)
  2003-10-15 12:42 ` Andi Kleen
@ 2003-10-15 12:54 ` Christoph Hellwig
  2003-10-15 13:29 ` Matthew Wilcox
                   ` (10 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2003-10-15 12:54 UTC (permalink / raw)
  To: linux-ia64

On Wed, Oct 15, 2003 at 11:25:04AM +1000, Nathan Scott wrote:
> I'd appreciate any suggestions about a cleaner way to do the
> common types.h and nfsd/syscall.h header changes.

First question:  What about other LP64 arches besides IA64.  IMHO
having differen't ino_t on different LP64 arches is a rather bad idea.

AQnd if we go the ino_t is unsigned long route for all 64bit arches
the ifdefs would also magically disappear..


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (21 preceding siblings ...)
  2003-10-15 12:54 ` Christoph Hellwig
@ 2003-10-15 13:29 ` Matthew Wilcox
  2003-10-15 13:40 ` Christoph Hellwig
                   ` (9 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Matthew Wilcox @ 2003-10-15 13:29 UTC (permalink / raw)
  To: linux-ia64

On Wed, Oct 15, 2003 at 02:54:53PM +0200, Christoph Hellwig wrote:
> On Wed, Oct 15, 2003 at 11:25:04AM +1000, Nathan Scott wrote:
> > I'd appreciate any suggestions about a cleaner way to do the
> > common types.h and nfsd/syscall.h header changes.
> 
> First question:  What about other LP64 arches besides IA64.  IMHO
> having differen't ino_t on different LP64 arches is a rather bad idea.

All architectures typedef __kernel_ino_t to unsigned long except alpha,
ia64 and s390x.  I guess ia64 just copied the wrong architecture at
the beginning.

> AQnd if we go the ino_t is unsigned long route for all 64bit arches
> the ifdefs would also magically disappear..

Sounds like a good plan.

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (22 preceding siblings ...)
  2003-10-15 13:29 ` Matthew Wilcox
@ 2003-10-15 13:40 ` Christoph Hellwig
  2003-10-15 16:32 ` David Mosberger
                   ` (8 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2003-10-15 13:40 UTC (permalink / raw)
  To: linux-ia64

On Wed, Oct 15, 2003 at 02:29:21PM +0100, Matthew Wilcox wrote:
> On Wed, Oct 15, 2003 at 02:54:53PM +0200, Christoph Hellwig wrote:
> > On Wed, Oct 15, 2003 at 11:25:04AM +1000, Nathan Scott wrote:
> > > I'd appreciate any suggestions about a cleaner way to do the
> > > common types.h and nfsd/syscall.h header changes.
> > 
> > First question:  What about other LP64 arches besides IA64.  IMHO
> > having differen't ino_t on different LP64 arches is a rather bad idea.
> 
> All architectures typedef __kernel_ino_t to unsigned long except alpha,
> ia64 and s390x.  I guess ia64 just copied the wrong architecture at
> the beginning.

So we should probably change introduce __HAVE_BROKEN_INO_T for alpha
and s390x?  Even better those should get new versions of the relevant
syscalls, I expect at least s390x to want them one day.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (23 preceding siblings ...)
  2003-10-15 13:40 ` Christoph Hellwig
@ 2003-10-15 16:32 ` David Mosberger
  2003-10-15 16:59 ` David Mosberger
                   ` (7 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: David Mosberger @ 2003-10-15 16:32 UTC (permalink / raw)
  To: linux-ia64

>>>>> On Wed, 15 Oct 2003 16:34:09 +1000, Nathan Scott <nathans@sgi.com> said:

  Nathan> On Tue, Oct 14, 2003 at 11:21:09PM -0700, David Mosberger
  Nathan> wrote:
  >> >>>>> On Wed, 15 Oct 2003 16:06:02 +1000, Nathan Scott
  >> <nathans@sgi.com> said:

  Nathan> It turns out that neither is a problem for us in practice.
  >>  Sounds find to me, then.  Except, I'd replace #ifdef __ia64__
  >> with #ifdef CONFIG_IA64, so you're relying (less) on compiler
  >> magic.


  Nathan> Oh.  I had avoided that because it requires any sources
  Nathan> including these headers to have already included
  Nathan> linux/config.h, which they may not be doing.  linux/types.h
  Nathan> is included by userspace code too, I believe, so may be an
  Nathan> issue there too.

OK.

Upon further investigation, I found that glibc defines its own "struct
ustat" and, guess what, it already declare __ino_t as unsigned long:

 (gdb) ptype struct ustat
 type = struct ustat {
     __daddr_t f_tfree;
     __ino_t f_tinode;
     char f_fname[6];
     char f_fpack[6];
 }
 (gdb) ptype __ino_t
 type = long unsigned int

So there is no need to have that ugly #ifdef for struct ustat.

I'm not 100% sure yet what to do about NFS.

	--david

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (24 preceding siblings ...)
  2003-10-15 16:32 ` David Mosberger
@ 2003-10-15 16:59 ` David Mosberger
  2003-10-15 17:40 ` David Mosberger
                   ` (6 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: David Mosberger @ 2003-10-15 16:59 UTC (permalink / raw)
  To: linux-ia64

>>>>> On Tue, 14 Oct 2003 22:18:10 -0700, Andrew Morton <akpm@osdl.org> said:

  Andrew> David Mosberger <davidm@napali.hpl.hp.com> wrote:

  >>  NFS is nastier (ugly multiplexed syscalls...).

  Andrew> Well I think that whole interface is deprecated anyway and
  Andrew> the new nfsutils doesn't use it - it uses exportfs instead.

  Andrew> You'd need to check with Neil, but if I'm right you could
  Andrew> just say "use current nfsutils on ia64".

Ugh, looks like there may be some problems with exportfs itself?
I see:

static struct dentry *get_object(struct super_block *sb, void *vobjp)
{
	__u32 *objp = vobjp;
	unsigned long ino = objp[0];
	__u32 generation = objp[1];

	return export_iget(sb, ino, generation);
}

So it looks to me like exportfs supports only 32-bit ino_t?

(Neil, to give you some background: we'd like to change ino_t on ia64
from 32 to 64 bits and found that the only potential ABI issue is due
to NFS; "struct nfsctl_export" is definitely an issue, but perhaps we
can live with that.  I'm less certain about any issues exportfs might
have.)

	--david

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (25 preceding siblings ...)
  2003-10-15 16:59 ` David Mosberger
@ 2003-10-15 17:40 ` David Mosberger
  2003-10-15 23:40 ` Neil Brown
                   ` (5 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: David Mosberger @ 2003-10-15 17:40 UTC (permalink / raw)
  To: linux-ia64

Nathan,

Based on the previous discussions, I applied your patch to change
ino_t to "unsigned long".  This fixes ustat() (for glibc) and breaks
nfsctl_export, but with reasonably recent nfsutils, that broken call
won't be used and exportfs seems to work fine (at least I was able to
mount an NFS filesystem from the patched box just fine).

Thanks,

	--david

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (26 preceding siblings ...)
  2003-10-15 17:40 ` David Mosberger
@ 2003-10-15 23:40 ` Neil Brown
  2003-10-16  1:20 ` David Mosberger
                   ` (4 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Neil Brown @ 2003-10-15 23:40 UTC (permalink / raw)
  To: linux-ia64

On Wednesday October 15, davidm@napali.hpl.hp.com wrote:
> >>>>> On Tue, 14 Oct 2003 22:18:10 -0700, Andrew Morton <akpm@osdl.org> said:
> 
>   Andrew> David Mosberger <davidm@napali.hpl.hp.com> wrote:
> 
>   >>  NFS is nastier (ugly multiplexed syscalls...).
> 
>   Andrew> Well I think that whole interface is deprecated anyway and
>   Andrew> the new nfsutils doesn't use it - it uses exportfs instead.
> 
>   Andrew> You'd need to check with Neil, but if I'm right you could
>   Andrew> just say "use current nfsutils on ia64".
> 
> Ugh, looks like there may be some problems with exportfs itself?
> I see:
> 
> static struct dentry *get_object(struct super_block *sb, void *vobjp)
> {
> 	__u32 *objp = vobjp;
> 	unsigned long ino = objp[0];
> 	__u32 generation = objp[1];
> 
> 	return export_iget(sb, ino, generation);
> }
> 
> So it looks to me like exportfs supports only 32-bit ino_t?

Looks can be deceiving (especially when those /* */ operators haven't
been used correctly).

get_object is part of the legacy support in exportfs, where legacy
really means ext2/ext3.
Any filesystem that actually used 64bit inode numbers would need to
define it's own export_operations->get_dentry function which can
interpret the file handle however it likes, including using 64 bits of
it for an inode number (though such a filesystem probably wouldn't
work with NFSv2, but that's OK).

> 
> (Neil, to give you some background: we'd like to change ino_t on ia64
> from 32 to 64 bits and found that the only potential ABI issue is due
> to NFS; "struct nfsctl_export" is definitely an issue, but perhaps we
> can live with that.  I'm less certain about any issues exportfs might
> have.)

ex_ino in nfsctl_export is not actually used, so all that is needed is
to make sure user_space and kernel_space agree on the side of the
field.
Maybe a __kernel_old_ino_t or soemthing.
But I would be quite happy to #ifdef out that system call for ia64 if
that was preferred.


So in short: there is no big problem, and the small problem is largely
   one of aesthetics.

NeilBrown

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (27 preceding siblings ...)
  2003-10-15 23:40 ` Neil Brown
@ 2003-10-16  1:20 ` David Mosberger
  2003-10-16 22:47 ` Nathan Scott
                   ` (3 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: David Mosberger @ 2003-10-16  1:20 UTC (permalink / raw)
  To: linux-ia64

>>>>> On Thu, 16 Oct 2003 09:40:28 +1000, Neil Brown <neilb@cse.unsw.edu.au> said:

  >> So it looks to me like exportfs supports only 32-bit ino_t?

  Neil> Looks can be deceiving (especially when those /* */ operators
  Neil> haven't been used correctly).

  Neil> get_object is part of the legacy support in exportfs, where
  Neil> legacy really means ext2/ext3.  Any filesystem that actually
  Neil> used 64bit inode numbers would need to define it's own
  Neil> export_operations->get_dentry function which can interpret the
  Neil> file handle however it likes, including using 64 bits of it
  Neil> for an inode number (though such a filesystem probably
  Neil> wouldn't work with NFSv2, but that's OK).

Ah, that sounds good then.

  >>  (Neil, to give you some background: we'd like to change ino_t on
  >> ia64 from 32 to 64 bits and found that the only potential ABI
  >> issue is due to NFS; "struct nfsctl_export" is definitely an
  >> issue, but perhaps we can live with that.  I'm less certain about
  >> any issues exportfs might have.)

  Neil> ex_ino in nfsctl_export is not actually used, so all that is
  Neil> needed is to make sure user_space and kernel_space agree on
  Neil> the side of the field.  Maybe a __kernel_old_ino_t or
  Neil> soemthing.  But I would be quite happy to #ifdef out that
  Neil> system call for ia64 if that was preferred.

  Neil> So in short: there is no big problem, and the small problem is
  Neil> largely one of aesthetics.

OK, perhaps for now you could use Nathan's #ifdef __ia64__?  Something
along the lines of __kernel_old_ino_t certainly would be cleaner, if
other arches need something similar.

	--daviad

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (28 preceding siblings ...)
  2003-10-16  1:20 ` David Mosberger
@ 2003-10-16 22:47 ` Nathan Scott
  2003-10-17  0:47 ` Neil Brown
                   ` (2 subsequent siblings)
  32 siblings, 0 replies; 34+ messages in thread
From: Nathan Scott @ 2003-10-16 22:47 UTC (permalink / raw)
  To: linux-ia64

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

Hi all,

On Wed, Oct 15, 2003 at 06:20:25PM -0700, David Mosberger wrote:
>   >>  (Neil, to give you some background: we'd like to change ino_t on
>   >> ia64 from 32 to 64 bits and found that the only potential ABI
>   >> issue is due to NFS; "struct nfsctl_export" is definitely an
>   >> issue, but perhaps we can live with that.  I'm less certain about
>   >> any issues exportfs might have.)
> 
>   Neil> ex_ino in nfsctl_export is not actually used, so all that is
>   Neil> needed is to make sure user_space and kernel_space agree on
>   Neil> the side of the field.  Maybe a __kernel_old_ino_t or
>   Neil> soemthing.  But I would be quite happy to #ifdef out that
>   Neil> system call for ia64 if that was preferred.
> 
>   Neil> So in short: there is no big problem, and the small problem is
>   Neil> largely one of aesthetics.
> 
> OK, perhaps for now you could use Nathan's #ifdef __ia64__?  Something
> along the lines of __kernel_old_ino_t certainly would be cleaner, if
> other arches need something similar.
> 

What's your preference here Neil?  I'll make a patch to create
__kernel_old_ino_t for each arch and nfsd/syscall.h if you like
that approach, else these patches seem to be all we need for ABI
compatibility now.

thanks.

-- 
Nathan

[-- Attachment #2: ino_t-2.6.patch --]
[-- Type: text/plain, Size: 1265 bytes --]


===========================================================================
linux/include/asm-ia64/posix_types.h
===========================================================================

--- /usr/tmp/TmpDir.2032422-0/linux/include/asm-ia64/posix_types.h_1.4	Wed Oct 15 07:57:15 2003
+++ linux/include/asm-ia64/posix_types.h	Tue Oct 14 16:23:17 2003
@@ -10,7 +10,7 @@
  *	David Mosberger-Tang <davidm@hpl.hp.com>
  */
 
-typedef unsigned int	__kernel_ino_t;
+typedef unsigned long	__kernel_ino_t;
 typedef unsigned int	__kernel_mode_t;
 typedef unsigned int	__kernel_nlink_t;
 typedef long		__kernel_off_t;

===========================================================================
linux/include/linux/nfsd/syscall.h
===========================================================================

--- /usr/tmp/TmpDir.2032422-0/linux/include/linux/nfsd/syscall.h_1.11	Wed Oct 15 07:57:15 2003
+++ linux/include/linux/nfsd/syscall.h	Tue Oct 14 16:18:07 2003
@@ -60,7 +60,11 @@
 	char			ex_client[NFSCLNT_IDMAX+1];
 	char			ex_path[NFS_MAXPATHLEN+1];
 	__kernel_old_dev_t	ex_dev;
+#ifdef __ia64__
+	unsigned int		ex_ino;	/* ABI preservation */
+#else
 	__kernel_ino_t		ex_ino;
+#endif
 	int			ex_flags;
 	__kernel_uid_t		ex_anon_uid;
 	__kernel_gid_t		ex_anon_gid;


[-- Attachment #3: ino_t-2.4.patch --]
[-- Type: text/plain, Size: 1478 bytes --]


===========================================================================
linux/include/asm-ia64/posix_types.h
===========================================================================

--- /usr/tmp/TmpDir.14060-0/linux/include/asm-ia64/posix_types.h_1.1	Wed Oct 15 09:13:59 2003
+++ linux/include/asm-ia64/posix_types.h	Fri Oct 10 11:48:09 2003
@@ -11,7 +11,7 @@
  */
 
 typedef unsigned int	__kernel_dev_t;
-typedef unsigned int	__kernel_ino_t;
+typedef unsigned long	__kernel_ino_t;
 typedef unsigned int	__kernel_mode_t;
 typedef unsigned int	__kernel_nlink_t;
 typedef long		__kernel_off_t;

===========================================================================
linux/include/linux/nfsd/syscall.h
===========================================================================

--- /usr/tmp/TmpDir.14060-0/linux/include/linux/nfsd/syscall.h_1.6	Wed Oct 15 09:13:59 2003
+++ linux/include/linux/nfsd/syscall.h	Tue Oct 14 16:06:11 2003
@@ -60,7 +60,11 @@
 	char			ex_client[NFSCLNT_IDMAX+1];
 	char			ex_path[NFS_MAXPATHLEN+1];
 	__kernel_dev_t		ex_dev;
+#ifdef __ia64__
+	unsigned int		ex_ino;	/* ABI preservation */
+#else
 	__kernel_ino_t		ex_ino;
+#endif
 	int			ex_flags;
 	__kernel_uid_t		ex_anon_uid;
 	__kernel_gid_t		ex_anon_gid;
@@ -81,7 +85,11 @@
 struct nfsctl_fhparm {
 	struct sockaddr		gf_addr;
 	__kernel_dev_t		gf_dev;
+#ifdef __ia64__
+	unsigned int		gf_ino;	/* ABI preservation */
+#else
 	__kernel_ino_t		gf_ino;
+#endif
 	int			gf_version;
 };
 


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (29 preceding siblings ...)
  2003-10-16 22:47 ` Nathan Scott
@ 2003-10-17  0:47 ` Neil Brown
  2003-10-17  1:56 ` Nathan Scott
  2003-10-21  3:37 ` Neil Brown
  32 siblings, 0 replies; 34+ messages in thread
From: Neil Brown @ 2003-10-17  0:47 UTC (permalink / raw)
  To: linux-ia64

On Friday October 17, nathans@sgi.com wrote:
> > 
> 
> What's your preference here Neil?  I'll make a patch to create
> __kernel_old_ino_t for each arch and nfsd/syscall.h if you like
> that approach, else these patches seem to be all we need for ABI
> compatibility now.
> 
> thanks.

I don't really want __kernel_old_ino_t as it would clutter every arch
just for nfsd, and just for 2.6.

I think I would like

#ifdef __ia64__
 typedef unsigned int __nfsd_ino_t;
#else
 typedef __kernel_ino_t __nfsd_ino_t;
#endif

at the top of syscall.h, and then use __nfsd_ino_t where needed.
This would keep all the 'clutter' inside nfsd, and in one place, and I
can remove it all when I discard that system call in 2.7.

If other arch's want to change their __kernel_ino_t, then we just need
to add a make a change within that #if/#endif

NeilBrown

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (30 preceding siblings ...)
  2003-10-17  0:47 ` Neil Brown
@ 2003-10-17  1:56 ` Nathan Scott
  2003-10-21  3:37 ` Neil Brown
  32 siblings, 0 replies; 34+ messages in thread
From: Nathan Scott @ 2003-10-17  1:56 UTC (permalink / raw)
  To: linux-ia64

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

hi Neil,

On Fri, Oct 17, 2003 at 10:47:14AM +1000, Neil Brown wrote:
> I don't really want __kernel_old_ino_t as it would clutter every arch
> just for nfsd, and just for 2.6.

OK sounds good, although we'd like these changes in 2.4 too
(eventually).  Do these patches look acceptable?

thanks.

-- 
Nathan

[-- Attachment #2: nfsd-ia64-ino_t-2.6.patch --]
[-- Type: text/plain, Size: 792 bytes --]

--- /usr/tmp/TmpDir.1899229-0/linux/include/linux/nfsd/syscall.h_1.11	Fri Oct 17 11:42:05 2003
+++ linux/include/linux/nfsd/syscall.h	Fri Oct 17 11:13:35 2003
@@ -22,6 +22,16 @@
 #include <linux/nfsd/auth.h>
 
 /*
+ * ABI compatibility type definition for the IA64 __kernel_ino_t.
+ * This can be removed along with the nfsctl_export syscall (2.7).
+ */
+#ifdef __ia64__
+ typedef unsigned int	__nfsd_ino_t;
+#else
+ typedef __kernel_ino_t	__nfsd_ino_t;
+#endif
+
+/*
  * Version of the syscall interface
  */
 #define NFSCTL_VERSION		0x0201
@@ -60,7 +70,7 @@
 	char			ex_client[NFSCLNT_IDMAX+1];
 	char			ex_path[NFS_MAXPATHLEN+1];
 	__kernel_old_dev_t	ex_dev;
-	__kernel_ino_t		ex_ino;
+	__nfsd_ino_t		ex_ino;
 	int			ex_flags;
 	__kernel_uid_t		ex_anon_uid;
 	__kernel_gid_t		ex_anon_gid;

[-- Attachment #3: nfsd-ia64-ino_t-2.4.patch --]
[-- Type: text/plain, Size: 997 bytes --]

--- /usr/tmp/TmpDir.1959-0/linux/include/linux/nfsd/syscall.h_1.6	Fri Oct 17 11:56:38 2003
+++ linux/include/linux/nfsd/syscall.h	Fri Oct 17 11:56:14 2003
@@ -22,6 +22,17 @@
 #include <linux/nfsd/auth.h>
 
 /*
+ * ABI compatibility type definition for the IA64 __kernel_ino_t.
+ * This can be removed along with the nfsctl_export syscall (2.7)
+ * and the nfsctl_fhparm syscall (2.5).
+ */
+#ifdef __ia64__
+ typedef unsigned int	__nfsd_ino_t;
+#else
+ typedef __kernel_ino_t	__nfsd_ino_t;
+#endif
+
+/*
  * Version of the syscall interface
  */
 #define NFSCTL_VERSION		0x0201
@@ -60,7 +71,7 @@
 	char			ex_client[NFSCLNT_IDMAX+1];
 	char			ex_path[NFS_MAXPATHLEN+1];
 	__kernel_dev_t		ex_dev;
-	__kernel_ino_t		ex_ino;
+	__nfsd_ino_t		ex_ino;
 	int			ex_flags;
 	__kernel_uid_t		ex_anon_uid;
 	__kernel_gid_t		ex_anon_gid;
@@ -81,7 +92,7 @@
 struct nfsctl_fhparm {
 	struct sockaddr		gf_addr;
 	__kernel_dev_t		gf_dev;
-	__kernel_ino_t		gf_ino;
+	__nfsd_ino_t		gf_ino;
 	int			gf_version;
 };
 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: IA64 ino_t incorrectly sized?
  2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
                   ` (31 preceding siblings ...)
  2003-10-17  1:56 ` Nathan Scott
@ 2003-10-21  3:37 ` Neil Brown
  32 siblings, 0 replies; 34+ messages in thread
From: Neil Brown @ 2003-10-21  3:37 UTC (permalink / raw)
  To: linux-ia64

On Friday October 17, nathans@sgi.com wrote:
> hi Neil,
> 
> On Fri, Oct 17, 2003 at 10:47:14AM +1000, Neil Brown wrote:
> > I don't really want __kernel_old_ino_t as it would clutter every arch
> > just for nfsd, and just for 2.6.
> 
> OK sounds good, although we'd like these changes in 2.4 too
> (eventually).  Do these patches look acceptable?
> 

Yes, I am happy with those.

NeilBrown

> thanks.
> 
> -- 
> Nathan
> --- /usr/tmp/TmpDir.1899229-0/linux/include/linux/nfsd/syscall.h_1.11	Fri Oct 17 11:42:05 2003
> +++ linux/include/linux/nfsd/syscall.h	Fri Oct 17 11:13:35 2003
> @@ -22,6 +22,16 @@
>  #include <linux/nfsd/auth.h>
>  
>  /*
> + * ABI compatibility type definition for the IA64 __kernel_ino_t.
> + * This can be removed along with the nfsctl_export syscall (2.7).
> + */
> +#ifdef __ia64__
> + typedef unsigned int	__nfsd_ino_t;
> +#else
> + typedef __kernel_ino_t	__nfsd_ino_t;
> +#endif
> +
> +/*
>   * Version of the syscall interface
>   */
>  #define NFSCTL_VERSION		0x0201
> @@ -60,7 +70,7 @@
>  	char			ex_client[NFSCLNT_IDMAX+1];
>  	char			ex_path[NFS_MAXPATHLEN+1];
>  	__kernel_old_dev_t	ex_dev;
> -	__kernel_ino_t		ex_ino;
> +	__nfsd_ino_t		ex_ino;
>  	int			ex_flags;
>  	__kernel_uid_t		ex_anon_uid;
>  	__kernel_gid_t		ex_anon_gid;
> --- /usr/tmp/TmpDir.1959-0/linux/include/linux/nfsd/syscall.h_1.6	Fri Oct 17 11:56:38 2003
> +++ linux/include/linux/nfsd/syscall.h	Fri Oct 17 11:56:14 2003
> @@ -22,6 +22,17 @@
>  #include <linux/nfsd/auth.h>
>  
>  /*
> + * ABI compatibility type definition for the IA64 __kernel_ino_t.
> + * This can be removed along with the nfsctl_export syscall (2.7)
> + * and the nfsctl_fhparm syscall (2.5).
> + */
> +#ifdef __ia64__
> + typedef unsigned int	__nfsd_ino_t;
> +#else
> + typedef __kernel_ino_t	__nfsd_ino_t;
> +#endif
> +
> +/*
>   * Version of the syscall interface
>   */
>  #define NFSCTL_VERSION		0x0201
> @@ -60,7 +71,7 @@
>  	char			ex_client[NFSCLNT_IDMAX+1];
>  	char			ex_path[NFS_MAXPATHLEN+1];
>  	__kernel_dev_t		ex_dev;
> -	__kernel_ino_t		ex_ino;
> +	__nfsd_ino_t		ex_ino;
>  	int			ex_flags;
>  	__kernel_uid_t		ex_anon_uid;
>  	__kernel_gid_t		ex_anon_gid;
> @@ -81,7 +92,7 @@
>  struct nfsctl_fhparm {
>  	struct sockaddr		gf_addr;
>  	__kernel_dev_t		gf_dev;
> -	__kernel_ino_t		gf_ino;
> +	__nfsd_ino_t		gf_ino;
>  	int			gf_version;
>  };
>  

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2003-10-21  3:37 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-09-17  7:10 IA64 ino_t incorrectly sized? Nathan Scott
2003-09-17 14:33 ` Jes Sorensen
2003-09-17 17:26 ` David Mosberger
2003-09-29  5:52 ` Nathan Scott
2003-10-08 23:51 ` David Mosberger
2003-10-09  1:25 ` Nathan Scott
2003-10-09  1:57 ` David Mosberger
2003-10-09  3:15 ` Nathan Scott
2003-10-09  3:53 ` David Mosberger
2003-10-09  4:55 ` Nathan Scott
2003-10-09 20:46 ` David Mosberger
2003-10-10  2:22 ` Nathan Scott
2003-10-15  1:25 ` Nathan Scott
2003-10-15  1:48 ` Andrew Morton
2003-10-15  4:47 ` David Mosberger
2003-10-15  5:18 ` Andrew Morton
2003-10-15  6:06 ` Nathan Scott
2003-10-15  6:16 ` Nathan Scott
2003-10-15  6:21 ` David Mosberger
2003-10-15  6:28 ` Andrew Morton
2003-10-15  6:34 ` Nathan Scott
2003-10-15 12:42 ` Andi Kleen
2003-10-15 12:54 ` Christoph Hellwig
2003-10-15 13:29 ` Matthew Wilcox
2003-10-15 13:40 ` Christoph Hellwig
2003-10-15 16:32 ` David Mosberger
2003-10-15 16:59 ` David Mosberger
2003-10-15 17:40 ` David Mosberger
2003-10-15 23:40 ` Neil Brown
2003-10-16  1:20 ` David Mosberger
2003-10-16 22:47 ` Nathan Scott
2003-10-17  0:47 ` Neil Brown
2003-10-17  1:56 ` Nathan Scott
2003-10-21  3:37 ` Neil Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.