[PATCH] 2.6.8-rc1 including AFS in ia64 and other ARCHS builds breaks the compilation

[PATCH] 2.6.8-rc1 including AFS in ia64 and other ARCHS builds breaks the compilation

[Darren Williams]

Including Andrew File System on any arch other
than i386 and x86_64 will break the compilation
due to the use of 'struct_cpy()', which is only
define in the two archs above and both archs
define it differently:
i386:
#define struct_cpy(x,y) 			\
({						\
	if (sizeof(*(x)) != sizeof(*(y))) 	\
		__struct_cpy_bug();		\
	memcpy(x, y, sizeof(*(x)));		\
})

x86_64:
#define struct_cpy(x,y) (*(x)=*(y))

both in include/asm/string.h

A quick discussion here suggests that we are not
doing a deep copy of the struct though others
may by able to enlighten us on what happens to
pointers within a struct?

I have applied the i386 definition to ia64 and
compiles OK, though I cannot test it since I
do not have direct access to AFS. 

diff -Nru a/include/asm-ia64/string.h b/include/asm-ia64/string.h
--- a/include/asm-ia64/string.h 2004-07-14 10:54:17 +10:00
+++ b/include/asm-ia64/string.h 2004-07-14 10:54:17 +10:00
@@ -20,4 +20,19 @@
 extern void *memcpy (void *, const void *, __kernel_size_t);
 extern void *memset (void *, int, __kernel_size_t);
  
+
+/*
+ * struct_cpy(x,y), copy structure *x into (matching structure) *y.
+ *
+ * We get link-time errors if the structure sizes do not match.
+ * There is no runtime overhead, it's all optimized away at
+ * compile time.
+ */
+#define struct_cpy(x,y)                        \
+({                                             \
+       if (sizeof(*(x)) != sizeof(*(y)))       \
+               __struct_cpy_bug();             \
+       memcpy(x, y, sizeof(*(x)));             \
+})
+
 #endif /* _ASM_IA64_STRING_H */

--------------------------------------------------
Darren Williams <dsw AT gelato.unsw.edu.au>
Gelato@UNSW <www.gelato.unsw.edu.au>
--------------------------------------------------

Re: [PATCH] 2.6.8-rc1 fix AFS struct_cpy use which break !X86

[Vincent Hanquez]

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

On Wed, Jul 14, 2004 at 11:07:06AM +1000, Darren Williams wrote:
> Including Andrew File System on any arch other
> than i386 and x86_64 will break the compilation
> due to the use of 'struct_cpy()', which is only
> define in the two archs above and both archs
> define it differently:

struct_cpy will be remove as suggest by AKPM in a recent discussion in LKML.

> A quick discussion here suggests that we are not
> doing a deep copy of the struct though others
> may by able to enlighten us on what happens to
> pointers within a struct?

using only *x = *y works.

> I have applied the i386 definition to ia64 and
> compiles OK, though I cannot test it since I
> do not have direct access to AFS. 

Ok, what about the following patch ?

Index: fs/afs/mntpt.c
===================================================================
--- fs/afs/mntpt.c	(revision 1)
+++ fs/afs/mntpt.c	(working copy)
@@ -257,7 +257,7 @@
 	if (IS_ERR(newmnt))
 		return PTR_ERR(newmnt);
 
-	struct_cpy(&newnd, nd);
+	newnd = *nd;
 	newnd.dentry = dentry;
 	err = do_add_mount(newmnt, &newnd, 0, &afs_vfsmounts);
 
Index: fs/afs/vlocation.c
===================================================================
--- fs/afs/vlocation.c	(revision 1)
+++ fs/afs/vlocation.c	(working copy)
@@ -906,7 +906,7 @@
 		if (!vlocation->valid ||
 		    vlocation->vldb.rtime == vldb->rtime
 		    ) {
-			struct_cpy(&vlocation->vldb, vldb);
+			vlocation->vldb = *vldb;
 			vlocation->valid = 1;
 			_leave(" = SUCCESS [c->m]");
 			return CACHEFS_MATCH_SUCCESS;
@@ -947,7 +947,7 @@
 
 	_enter("");
 
-	struct_cpy(vldb,&vlocation->vldb);
+	*vldb = vlocation->vldb;
 
 } /* end afs_vlocation_cache_update() */
 #endif

-- 
Tab

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] 2.6.8-rc1 fix AFS struct_cpy use which break !X86

[Darren Williams]

Yep, that fixed it, we'll wait for the original patch to be merged.

see
http://lkml.org/lkml/2004/7/12/176
for the original thread and patch.

Thanks
Darren

> Ok, what about the following patch ?
> 
> Index: fs/afs/mntpt.c
> ===================================================================
> --- fs/afs/mntpt.c	(revision 1)
> +++ fs/afs/mntpt.c	(working copy)
> @@ -257,7 +257,7 @@
>  	if (IS_ERR(newmnt))
>  		return PTR_ERR(newmnt);
>  
> -	struct_cpy(&newnd, nd);
> +	newnd = *nd;
>  	newnd.dentry = dentry;
>  	err = do_add_mount(newmnt, &newnd, 0, &afs_vfsmounts);
>  
> Index: fs/afs/vlocation.c
> ===================================================================
> --- fs/afs/vlocation.c	(revision 1)
> +++ fs/afs/vlocation.c	(working copy)
> @@ -906,7 +906,7 @@
>  		if (!vlocation->valid ||
>  		    vlocation->vldb.rtime == vldb->rtime
>  		    ) {
> -			struct_cpy(&vlocation->vldb, vldb);
> +			vlocation->vldb = *vldb;
>  			vlocation->valid = 1;
>  			_leave(" = SUCCESS [c->m]");
>  			return CACHEFS_MATCH_SUCCESS;
> @@ -947,7 +947,7 @@
>  
>  	_enter("");
>  
> -	struct_cpy(vldb,&vlocation->vldb);
> +	*vldb = vlocation->vldb;
>  
>  } /* end afs_vlocation_cache_update() */
>  #endif


--------------------------------------------------
Darren Williams <dsw AT gelato.unsw.edu.au>
Gelato@UNSW <www.gelato.unsw.edu.au>
--------------------------------------------------

Re: [PATCH] 2.6.8-rc1 including AFS in ia64 and other ARCHS builds breaks the compilation

[David Howells]

Andrew Morton has a fix for this that involves removing struct_cpy() entirely
and going for direct assignment instead.

David