Re: 2.1.126 still no sparc64 autofsu

Re: 2.1.126 still no sparc64 autofsu

[H. Peter Anvin]

> Okay, I finally had time to take a closer look at this. I've
> found the problem but am not sure what's the best fix.
> 
> The linux/include/linux/auto_fs.h file is shared between the
> autofs.o kernel module and the automount user space daemon
> and contains the following struct:
> 
> struct autofs_packet_missing {
>        struct autofs_packet_hdr hdr;
>        unsigned long wait_queue_token;
>        int len;
>        char name[NAME_MAX+1];
> };
> 
> The problem is that on sparc64 systems the unsigned long
> generates a 64 bit long for the kernel to use, but the
> user space automount expects a 32 bit long and is therefore
> not able to reference the len and name fields properly. I've
> worked around the problem for now by inserting an unused
> long before wait_queue_token in the automount build.
> 
> I thought about providing a patch for the struct since it
> looks like 32 bits should be large enough for the token but
> was then told there are ioctl's which should handle passing
> data between 32 bit user <-> 64 bit kernel space automatically.
> I'm getting lost in the code trying to find them but if they
> exist I guess that's the best place for a fix.
> 
> Perhaps Dave M. or Jakub J. will have a look?
> 
> Thanks for the great work on the ultrapenguin guys. I'm
> really loving these ultra's with hme's at 100mb...
> 

Why is the "long" a different size in kernel and user space?  That
seems buggy.  It's an easy thing to fix, but it seems like the Wrong
Thing[TM].

Changing that to "int" would be just fine, *except* it would break the
protocol for all other platforms.

	-hpa

Re: 2.1.126 still no sparc64 autofsu

[Rich Sahlender]

H. Peter Anvin wrote:
> 
> Why is the "long" a different size in kernel and user space?  That
> seems buggy.  It's an easy thing to fix, but it seems like the Wrong
> Thing[TM].

I tend to agree it's buggy. I would expect the long to generate a 32
bit int even in the 64 bit kernel space unless you specifically ask
for a 64 bit long .ie. suns 'long long'. This would keep things
downward/backward compatible.

> 
> Changing that to "int" would be just fine, *except* it would break the
> protocol for all other platforms.
> 

I was going to provide a patch to do something like this, but unless I
misunderstood Alan I think he said there are ioctl's which can be used
to convert the 64 bit structs while passing to 32 bit user space. I'm
getting lost finding them though...

Anyway, I guess this is only a problem while were're still limited to
32 bit user space with 64 bit kernels.

Re: 2.1.126 still no sparc64 autofsu

[Jakub Jelinek]

> Why is the "long" a different size in kernel and user space?  That
> seems buggy.  It's an easy thing to fix, but it seems like the Wrong
> Thing[TM].

Simple reason: the userland application in question is a 32bit application,
not the native 64bit application, while the kernel is 64bit. 64bit apps
don't need any translations, 32bit apps run in 32bit emulation. So, we are
of course not going to change it to int, just add ioctls playing with that
structure into our 32bit compatibility layer.
> 
> Changing that to "int" would be just fine, *except* it would break the
> protocol for all other platforms.

Cheers,
    Jakub
___________________________________________________________________
Jakub Jelinek | jj@sunsite.mff.cuni.cz | http://sunsite.mff.cuni.cz
Administrator of SunSITE Czech Republic, MFF, Charles University
___________________________________________________________________
Ultralinux - first 64bit OS to take full power of the UltraSparc
Linux version 2.1.127 on a sparc64 machine (498.80 BogoMips).
___________________________________________________________________

Re: 2.1.126 still no sparc64 autofsu

[David S. Miller]

   From: "H. Peter Anvin" <hpa@transmeta.com>
   Date: 	Sat, 14 Nov 1998 00:17:49 -0800 (PST)

   Why is the "long" a different size in kernel and user space?  That
   seems buggy.  It's an easy thing to fix, but it seems like the
   Wrong Thing[TM].

   Changing that to "int" would be just fine, *except* it would break
   the protocol for all other platforms.

All of our userlands currently on sparc64 are 32-bit because we have
not bootstrapped a 64-bit userland fully enough yet.

We have a full (sic) 32-64 bit translation layer for all user visible
interfaces of the kernel already in files named
arch/sparc64/kernel/*32*.c which are supposed to take care of all
these issues.  Every once in a while an ioctl() or two slip by without
our noticing and we have to take care of it when the user finally hits
it.  :-)

Later,
David S. Miller
davem@dm.cobaltmicro.com

Re: 2.1.126 still no sparc64 autofsu

[H. Peter Anvin]

> 
> All of our userlands currently on sparc64 are 32-bit because we have
> not bootstrapped a 64-bit userland fully enough yet.
> 
> We have a full (sic) 32-64 bit translation layer for all user visible
> interfaces of the kernel already in files named
> arch/sparc64/kernel/*32*.c which are supposed to take care of all
> these issues.  Every once in a while an ioctl() or two slip by without
> our noticing and we have to take care of it when the user finally hits
> it.  :-)
> 

Problem is that autofs passes an "unsigned long" in the ioctl()
*argument*, not in a structure; also it passes data to the daemon via
a packetized protocol on a pipe.

Here is a patch that I think will help.  It should keep the protocol
unchanged on existing architectures (owing to the fact that
sizeof(int) = sizeof(long) on 32-bit machines) while letting SPARC
and MIPS (as far as I know the only hybrid 32/64-bit architectures)
run 32-bit binaries on a 64-bit kernel, using the fact that
sizeof(int) [32 bit] = sizeof(int) [64 bit]:

Please test this patch and give me feedback; if it works I'll pass it
on to Linus.

	-hpa

diff -ur stock/linux-2.1.128/fs/autofs/autofs_i.h linux-2.1.128-smp/fs/autofs/autofs_i.h
--- stock/linux-2.1.128/fs/autofs/autofs_i.h	Sun Nov 15 15:29:20 1998
+++ linux-2.1.128-smp/fs/autofs/autofs_i.h	Sun Nov 15 15:28:24 1998
@@ -66,9 +66,9 @@
 };
 
 struct autofs_wait_queue {
-	unsigned long wait_queue_token;
 	struct wait_queue *queue;
 	struct autofs_wait_queue *next;
+	autofs_wqt_t wait_queue_token;
 	/* We use the following to see what we are waiting for */
 	int hash;
 	int len;
@@ -79,8 +79,8 @@
 };
 
 struct autofs_symlink {
-	int len;
 	char *data;
+	int len;
 	time_t mtime;
 };
 
diff -ur stock/linux-2.1.128/fs/autofs/root.c linux-2.1.128-smp/fs/autofs/root.c
--- stock/linux-2.1.128/fs/autofs/root.c	Sun Nov 15 15:29:20 1998
+++ linux-2.1.128-smp/fs/autofs/root.c	Sun Nov 15 15:27:17 1998
@@ -112,7 +112,8 @@
 	if ( !(ent = autofs_hash_lookup(&sbi->dirhash, &dentry->d_name)) ) {
 		do {
 			if ( status && dentry->d_inode ) {
-				printk("autofs warning: lookup failure on positive dentry, status = %d, name = %s\n", status, dentry->d_name.name);
+				if ( status != -ENOENT )
+					printk("autofs warning: lookup failure on positive dentry, status = %d, name = %s\n", status, dentry->d_name.name);
 				return 0; /* Try to get the kernel to invalidate this dentry */
 			}
 
@@ -502,9 +503,9 @@
 	
 	switch(cmd) {
 	case AUTOFS_IOC_READY:	/* Wait queue: go ahead and retry */
-		return autofs_wait_release(sbi,arg,0);
+		return autofs_wait_release(sbi,(autofs_wqt_t)arg,0);
 	case AUTOFS_IOC_FAIL:	/* Wait queue: fail with ENOENT */
-		return autofs_wait_release(sbi,arg,-ENOENT);
+		return autofs_wait_release(sbi,(autofs_wqt_t)arg,-ENOENT);
 	case AUTOFS_IOC_CATATONIC: /* Enter catatonic mode (daemon shutdown) */
 		autofs_catatonic_mode(sbi);
 		return 0;
diff -ur stock/linux-2.1.128/fs/autofs/waitq.c linux-2.1.128-smp/fs/autofs/waitq.c
--- stock/linux-2.1.128/fs/autofs/waitq.c	Sun Nov 15 15:29:20 1998
+++ linux-2.1.128-smp/fs/autofs/waitq.c	Sun Nov 15 15:26:17 1998
@@ -18,7 +18,7 @@
 
 /* We make this a static variable rather than a part of the superblock; it
    is better if we don't reassign numbers easily even across filesystems */
-static int autofs_next_wait_queue = 1;
+static autofs_wqt_t autofs_next_wait_queue = 1;
 
 /* These are the signals we allow interrupting a pending mount */
 #define SHUTDOWN_SIGS	(sigmask(SIGKILL) | sigmask(SIGINT) | sigmask(SIGQUIT))
@@ -168,7 +168,7 @@
 }
 
 
-int autofs_wait_release(struct autofs_sb_info *sbi, unsigned long wait_queue_token, int status)
+int autofs_wait_release(struct autofs_sb_info *sbi, autofs_wqt_t wait_queue_token, int status)
 {
 	struct autofs_wait_queue *wq, **wql;
 
diff -ur stock/linux-2.1.128/include/linux/auto_fs.h linux-2.1.128-smp/include/linux/auto_fs.h
--- stock/linux-2.1.128/include/linux/auto_fs.h	Fri Oct 23 10:15:56 1998
+++ linux-2.1.128-smp/include/linux/auto_fs.h	Sun Nov 15 15:25:19 1998
@@ -22,6 +22,27 @@
 
 #define AUTOFS_PROTO_VERSION 3
 
+/*
+ * Architectures where both 32- and 64-bit binaries can be executed
+ * on 64-bit kernels need this.  This keeps the structure format
+ * uniform, and makes sure the wait_queue_token isn't too big to be
+ * passed back down to the kernel.
+ *
+ * This assumes that on these architectures:
+ * mode     32 bit    64 bit
+ * -------------------------
+ * int      32 bit    32 bit
+ * long     32 bit    64 bit
+ *
+ * If so, 32-bit user-space code should be backwards compatible.
+ */
+
+#if defined(__sparc__) || defined(__mips__)
+typedef unsigned int autofs_wqt_t;
+#else
+typedef unsigned long autofs_wqt_t;
+#endif
+
 enum autofs_packet_type {
 	autofs_ptype_missing,	/* Missing entry (mount request) */
 	autofs_ptype_expire,	/* Expire entry (umount request) */
@@ -34,7 +55,7 @@
 
 struct autofs_packet_missing {
 	struct autofs_packet_hdr hdr;
-        unsigned long wait_queue_token;
+        autofs_wqt_t wait_queue_token;
 	int len;
 	char name[NAME_MAX+1];
 };	

Re: 2.1.126 still no sparc64 autofsu

[David S. Miller]

   From: "H. Peter Anvin" <hpa@transmeta.com>
   Date: Sun, 15 Nov 1998 15:49:21 -0800 (PST)

   Problem is that autofs passes an "unsigned long" in the ioctl()
   *argument*, not in a structure; also it passes data to the daemon via
   a packetized protocol on a pipe.

Ok, we have a similar issue for rtnetlink on sparc64 running 32-bit
binaries.  That piece is so complex that I've decided to tell people
for now to wait until we have a 64-bit userland if they want to run
programs like gated :-)

I looked at this patch, and it occurs to me, why to make it an
unsigned long ever?  For one thing this wait_queue_token seems to be
set by an int counter (autofs_next_wait_queue).

Secondly, to avoid all kinds of problems in this area, the best way to
define structures which are a "protocol" over some transport (even a
pipe) is to use the available fixed typing schemes (u32, u16 etc.) the
kernel provides.  This makes issues like this never a problem because
a u32 in the kernel is the same as a u32 in userspace no matter if we
have a 32-bit binary running under emulation on a 64-bit kernel.

Do you agree? :-)

Later,
David S. Miller
davem@dm.cobaltmicro.com

Re: 2.1.126 still no sparc64 autofsu

[H. Peter Anvin]

> > Problem is that autofs passes an "unsigned long" in the ioctl()
> > *argument*, not in a structure; 
> 
> That's no problem. In fact, 32bit syscall entry code zero extends already in
> assembly, so handling this is really a nop.

Except when the value isn't zero in the high dword... then you have no
way to pass the value down.

> > also it passes data to the daemon via
> > a packetized protocol on a pipe.
> 
> That's the only problem I'd have with autofs and that's why it was not
> working. I was thinking about overriding the f_ops of the pipe file
> descriptor, so that it points to a special write method which would do the
> conversion of the structure before sending it over the pipe, but your
> autofs_wqt_t patch is of course much better, as it does not need additional 
> code and complexity. Thanks.

Does it work?  If so, then I can send it to Linus.

	-hpa

Re: 2.1.126 still no sparc64 autofsu

[Jakub Jelinek]

> Problem is that autofs passes an "unsigned long" in the ioctl()
> *argument*, not in a structure; 

That's no problem. In fact, 32bit syscall entry code zero extends already in
assembly, so handling this is really a nop.

> also it passes data to the daemon via
> a packetized protocol on a pipe.

That's the only problem I'd have with autofs and that's why it was not
working. I was thinking about overriding the f_ops of the pipe file
descriptor, so that it points to a special write method which would do the
conversion of the structure before sending it over the pipe, but your
autofs_wqt_t patch is of course much better, as it does not need additional 
code and complexity. Thanks.

Cheers,
    Jakub
___________________________________________________________________
Jakub Jelinek | jj@sunsite.mff.cuni.cz | http://sunsite.mff.cuni.cz
Administrator of SunSITE Czech Republic, MFF, Charles University
___________________________________________________________________
Ultralinux - first 64bit OS to take full power of the UltraSparc
Linux version 2.1.127 on a sparc64 machine (498.80 BogoMips).
___________________________________________________________________

Re: 2.1.126 still no sparc64 autofsu

[Jakub Jelinek]

> 
>    From: "H. Peter Anvin" <hpa@transmeta.com>
>    Date: Sun, 15 Nov 1998 15:49:21 -0800 (PST)
> 
>    Problem is that autofs passes an "unsigned long" in the ioctl()
>    *argument*, not in a structure; also it passes data to the daemon via
>    a packetized protocol on a pipe.
> 
> Ok, we have a similar issue for rtnetlink on sparc64 running 32-bit
> binaries.  That piece is so complex that I've decided to tell people
> for now to wait until we have a 64-bit userland if they want to run
> programs like gated :-)
> 
> I looked at this patch, and it occurs to me, why to make it an
> unsigned long ever?  For one thing this wait_queue_token seems to be
> set by an int counter (autofs_next_wait_queue).

I guess probably just for alpha compatibility. So the best would be to use a
u32 everywhere and have a special case for alpha - u64 - to keep it binary
compatible.

Cheers,
    Jakub
___________________________________________________________________
Jakub Jelinek | jj@sunsite.mff.cuni.cz | http://sunsite.mff.cuni.cz
Administrator of SunSITE Czech Republic, MFF, Charles University
___________________________________________________________________
Ultralinux - first 64bit OS to take full power of the UltraSparc
Linux version 2.1.127 on a sparc64 machine (498.80 BogoMips).
___________________________________________________________________

Re: 2.1.126 still no sparc64 autofsu

[H. Peter Anvin]

> > 
> > I looked at this patch, and it occurs to me, why to make it an
> > unsigned long ever?  For one thing this wait_queue_token seems to be
> > set by an int counter (autofs_next_wait_queue).
> 
> I guess probably just for alpha compatibility. So the best would be to use a
> u32 everywhere and have a special case for alpha - u64 - to keep it binary
> compatible.
> 
> Cheers,
>     Jakub

Actually, there is a simpler answer (I guess): Linus changed this one
while I was on vacation; it used to be u32.  :(

	-hpa

Re: 2.1.126 still no sparc64 autofsu

[Rich Sahlender]

I pulled the latest 2.1.129 from vger this morning and applied by
hand the parts of your patch that were not yet there. Got following
failure in "make modules" at fs/autofs/waitq.c:

waitq.c:172: conflicting types for `autofs_wait_release'
autofs_i.h:149: previous declaration of `autofs_wait_release'

So I changed the line in fs/autofs/autofs_i.h from
int autofs_wait_release(struct autofs_sb_info *,unsigned long,int);
to
int autofs_wait_release(struct autofs_sb_info *,autofs_wqt_t,int);

And all is well now on a sparc64!

Thanks guys,
Rich

PS... I caught up reading the rest of this thread, and changed
include/linux/auto_fs.h to use u32 in the typedef for autofs_wqt_t
instead of unsigned int which of course works too. Thanks again...

H. Peter Anvin wrote:
> > 
> > All of our userlands currently on sparc64 are 32-bit because we have
> > not bootstrapped a 64-bit userland fully enough yet.
> > 
> > We have a full (sic) 32-64 bit translation layer for all user visible
> > interfaces of the kernel already in files named
> > arch/sparc64/kernel/*32*.c which are supposed to take care of all
> > these issues.  Every once in a while an ioctl() or two slip by without
> > our noticing and we have to take care of it when the user finally hits
> > it.  :-)
> > 
> 
> Problem is that autofs passes an "unsigned long" in the ioctl()
> *argument*, not in a structure; also it passes data to the daemon via
> a packetized protocol on a pipe.
> 
> Here is a patch that I think will help.  It should keep the protocol
> unchanged on existing architectures (owing to the fact that
> sizeof(int) = sizeof(long) on 32-bit machines) while letting SPARC
> and MIPS (as far as I know the only hybrid 32/64-bit architectures)
> run 32-bit binaries on a 64-bit kernel, using the fact that
> sizeof(int) [32 bit] = sizeof(int) [64 bit]:
> 
> Please test this patch and give me feedback; if it works I'll pass it
> on to Linus.
> 
> 	-hpa
> 
> diff -ur stock/linux-2.1.128/fs/autofs/autofs_i.h linux-2.1.128-smp/fs/autofs/autofs_i.h
> --- stock/linux-2.1.128/fs/autofs/autofs_i.h	Sun Nov 15 15:29:20 1998
> +++ linux-2.1.128-smp/fs/autofs/autofs_i.h	Sun Nov 15 15:28:24 1998
> @@ -66,9 +66,9 @@
>  };
>  
>  struct autofs_wait_queue {
> -	unsigned long wait_queue_token;
>  	struct wait_queue *queue;
>  	struct autofs_wait_queue *next;
> +	autofs_wqt_t wait_queue_token;
>  	/* We use the following to see what we are waiting for */
>  	int hash;
>  	int len;
> @@ -79,8 +79,8 @@
>  };
>  
>  struct autofs_symlink {
> -	int len;
>  	char *data;
> +	int len;
>  	time_t mtime;
>  };
>  
> diff -ur stock/linux-2.1.128/fs/autofs/root.c linux-2.1.128-smp/fs/autofs/root.c
> --- stock/linux-2.1.128/fs/autofs/root.c	Sun Nov 15 15:29:20 1998
> +++ linux-2.1.128-smp/fs/autofs/root.c	Sun Nov 15 15:27:17 1998
> @@ -112,7 +112,8 @@
>  	if ( !(ent = autofs_hash_lookup(&sbi->dirhash, &dentry->d_name)) ) {
>  		do {
>  			if ( status && dentry->d_inode ) {
> -				printk("autofs warning: lookup failure on positive dentry, status = %d, name = %s\n", status, dentry->d_name.name);
> +				if ( status != -ENOENT )
> +					printk("autofs warning: lookup failure on positive dentry, status = %d, name = %s\n", status, dentry->d_name.name);
>  				return 0; /* Try to get the kernel to invalidate this dentry */
>  			}
>  
> @@ -502,9 +503,9 @@
>  	
>  	switch(cmd) {
>  	case AUTOFS_IOC_READY:	/* Wait queue: go ahead and retry */
> -		return autofs_wait_release(sbi,arg,0);
> +		return autofs_wait_release(sbi,(autofs_wqt_t)arg,0);
>  	case AUTOFS_IOC_FAIL:	/* Wait queue: fail with ENOENT */
> -		return autofs_wait_release(sbi,arg,-ENOENT);
> +		return autofs_wait_release(sbi,(autofs_wqt_t)arg,-ENOENT);
>  	case AUTOFS_IOC_CATATONIC: /* Enter catatonic mode (daemon shutdown) */
>  		autofs_catatonic_mode(sbi);
>  		return 0;
> diff -ur stock/linux-2.1.128/fs/autofs/waitq.c linux-2.1.128-smp/fs/autofs/waitq.c
> --- stock/linux-2.1.128/fs/autofs/waitq.c	Sun Nov 15 15:29:20 1998
> +++ linux-2.1.128-smp/fs/autofs/waitq.c	Sun Nov 15 15:26:17 1998
> @@ -18,7 +18,7 @@
>  
>  /* We make this a static variable rather than a part of the superblock; it
>     is better if we don't reassign numbers easily even across filesystems */
> -static int autofs_next_wait_queue = 1;
> +static autofs_wqt_t autofs_next_wait_queue = 1;
>  
>  /* These are the signals we allow interrupting a pending mount */
>  #define SHUTDOWN_SIGS	(sigmask(SIGKILL) | sigmask(SIGINT) | sigmask(SIGQUIT))
> @@ -168,7 +168,7 @@
>  }
>  
>  
> -int autofs_wait_release(struct autofs_sb_info *sbi, unsigned long wait_queue_token, int status)
> +int autofs_wait_release(struct autofs_sb_info *sbi, autofs_wqt_t wait_queue_token, int status)
>  {
>  	struct autofs_wait_queue *wq, **wql;
>  
> diff -ur stock/linux-2.1.128/include/linux/auto_fs.h linux-2.1.128-smp/include/linux/auto_fs.h
> --- stock/linux-2.1.128/include/linux/auto_fs.h	Fri Oct 23 10:15:56 1998
> +++ linux-2.1.128-smp/include/linux/auto_fs.h	Sun Nov 15 15:25:19 1998
> @@ -22,6 +22,27 @@
>  
>  #define AUTOFS_PROTO_VERSION 3
>  
> +/*
> + * Architectures where both 32- and 64-bit binaries can be executed
> + * on 64-bit kernels need this.  This keeps the structure format
> + * uniform, and makes sure the wait_queue_token isn't too big to be
> + * passed back down to the kernel.
> + *
> + * This assumes that on these architectures:
> + * mode     32 bit    64 bit
> + * -------------------------
> + * int      32 bit    32 bit
> + * long     32 bit    64 bit
> + *
> + * If so, 32-bit user-space code should be backwards compatible.
> + */
> +
> +#if defined(__sparc__) || defined(__mips__)
> +typedef unsigned int autofs_wqt_t;
> +#else
> +typedef unsigned long autofs_wqt_t;
> +#endif
> +
>  enum autofs_packet_type {
>  	autofs_ptype_missing,	/* Missing entry (mount request) */
>  	autofs_ptype_expire,	/* Expire entry (umount request) */
> @@ -34,7 +55,7 @@
>  
>  struct autofs_packet_missing {
>  	struct autofs_packet_hdr hdr;
> -        unsigned long wait_queue_token;
> +        autofs_wqt_t wait_queue_token;
>  	int len;
>  	char name[NAME_MAX+1];
>  };	
>