[PATCH] xfsprogs: fix unaligned access in libxfs

[PATCH] xfsprogs: fix unaligned access in libxfs

[Christoph Hellwig]

The get/put unaligned handlers we use to access the extent descriptor
are not good enough for architectures like Sparc that do not tolerate
dereferencing unaligned pointers.  Replace the implementation with the
one the kernel uses on these architectures.  It might be a tad
slower on architectures like x86, but I don't want to have multiple
implementations around to not let the testing matrix explode.
Also remove the unaligned.h header which includes another implementation
for unaligned access we don't actually use anymore.

Note that the little change to xfs_inode.c needs to go into the kernel
aswell, I will send a patch for that shortly.


Signed-off-by: Christoph Hellwig <hch@lst.de>
Reported-by: Gabriel Vlasiu <gabrielvlasiu@gmail.com>
Tested-by: Gabriel Vlasiu <gabrielvlasiu@gmail.com>

Index: xfsprogs-dev/libxfs/xfs.h
===================================================================
--- xfsprogs-dev.orig/libxfs/xfs.h	2009-08-27 10:06:13.377855006 -0300
+++ xfsprogs-dev/libxfs/xfs.h	2009-08-27 10:06:26.537884492 -0300
@@ -127,19 +127,37 @@ static inline int __do_div(unsigned long
 #define max_t(type,x,y) \
 	({ type __x = (x); type __y = (y); __x > __y ? __x: __y; })
 
-/* only 64 bit accesses used in xfs kernel code */
-static inline __u64 get_unaligned_be64(void *ptr)
+
+static inline __uint32_t __get_unaligned_be32(const __uint8_t *p)
+{
+        return p[0] << 24 | p[1] << 16 | p[2] << 8 | p[3];
+}
+
+static inline __uint64_t get_unaligned_be64(void *p)
 {
-	__be64	__tmp;
-	memmove(&__tmp, ptr, 8);
-	return be64_to_cpu(__tmp);
+	return (__uint64_t)__get_unaligned_be32(p) << 32 |
+			   __get_unaligned_be32(p + 4);
 }
 
-static inline void put_unaligned(__be64 val, void *ptr)
+static inline void __put_unaligned_be16(__uint16_t val, __uint8_t *p)
 {
-	memmove(ptr, &val, 8);
+	*p++ = val >> 8;
+	*p++ = val;
 }
 
+static inline void __put_unaligned_be32(__uint32_t val, __uint8_t *p)
+{
+	__put_unaligned_be16(val >> 16, p);
+	__put_unaligned_be16(val, p + 2);
+}
+
+static inline void put_unaligned_be64(__uint64_t val, void *p)
+{
+	__put_unaligned_be32(val >> 32, p);
+	__put_unaligned_be32(val, p + 4);
+}
+
+
 static inline __attribute__((const))
 int is_power_of_2(unsigned long n)
 {
Index: xfsprogs-dev/libxfs/xfs_inode.c
===================================================================
--- xfsprogs-dev.orig/libxfs/xfs_inode.c	2009-08-27 10:06:13.385884867 -0300
+++ xfsprogs-dev/libxfs/xfs_inode.c	2009-08-27 10:06:26.541855737 -0300
@@ -1056,8 +1056,8 @@ xfs_iextents_copy(
 		}
 
 		/* Translate to on disk format */
-		put_unaligned(cpu_to_be64(ep->l0), &dp->l0);
-		put_unaligned(cpu_to_be64(ep->l1), &dp->l1);
+		put_unaligned_be64(ep->l0, &dp->l0);
+		put_unaligned_be64(ep->l1, &dp->l1);
 		dp++;
 		copied++;
 	}
Index: xfsprogs-dev/include/Makefile
===================================================================
--- xfsprogs-dev.orig/include/Makefile	2009-08-28 12:29:42.257922053 -0300
+++ xfsprogs-dev/include/Makefile	2009-08-28 12:29:55.830456736 -0300
@@ -37,7 +37,7 @@ PHFILES = darwin.h freebsd.h irix.h linu
 DKHFILES = volume.h fstyp.h dvh.h
 LSRCFILES = $(shell echo $(PHFILES) | sed -e "s/$(PKG_PLATFORM).h//g")
 LSRCFILES += platform_defs.h.in builddefs.in buildmacros buildrules install-sh
-LSRCFILES += $(DKHFILES) command.h input.h path.h project.h unaligned.h
+LSRCFILES += $(DKHFILES) command.h input.h path.h project.h
 LDIRT = xfs disk
 
 default install: xfs disk
Index: xfsprogs-dev/include/unaligned.h
===================================================================
--- xfsprogs-dev.orig/include/unaligned.h	2009-08-28 12:29:34.710448280 -0300
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,120 +0,0 @@
-#ifndef _UNALIGNED_H_
-#define _UNALIGNED_H_
-
-/*
- * For the benefit of those who are trying to port Linux to another
- * architecture, here are some C-language equivalents. 
- *
- * This is based almost entirely upon Richard Henderson's
- * asm-alpha/unaligned.h implementation.  Some comments were
- * taken from David Mosberger's asm-ia64/unaligned.h header.
- */
-
-/* 
- * The main single-value unaligned transfer routines.
- */
-#define get_unaligned(ptr) \
-	__get_unaligned((ptr), sizeof(*(ptr)))
-#define put_unaligned(x,ptr) \
-	__put_unaligned((__u64)(x), (ptr), sizeof(*(ptr)))
-
-/*
- * This function doesn't actually exist.  The idea is that when
- * someone uses the macros below with an unsupported size (datatype),
- * the linker will alert us to the problem via an unresolved reference
- * error.
- */
-extern void bad_unaligned_access_length(void) __attribute__((noreturn));
-
-struct __una_u64 { __u64 x __attribute__((packed)); };
-struct __una_u32 { __u32 x __attribute__((packed)); };
-struct __una_u16 { __u16 x __attribute__((packed)); };
-
-/*
- * Elemental unaligned loads 
- */
-
-static inline __u64 __uldq(const __u64 *addr)
-{
-	const struct __una_u64 *ptr = (const struct __una_u64 *) addr;
-	return ptr->x;
-}
-
-static inline __u32 __uldl(const __u32 *addr)
-{
-	const struct __una_u32 *ptr = (const struct __una_u32 *) addr;
-	return ptr->x;
-}
-
-static inline __u16 __uldw(const __u16 *addr)
-{
-	const struct __una_u16 *ptr = (const struct __una_u16 *) addr;
-	return ptr->x;
-}
-
-/*
- * Elemental unaligned stores 
- */
-
-static inline void __ustq(__u64 val, __u64 *addr)
-{
-	struct __una_u64 *ptr = (struct __una_u64 *) addr;
-	ptr->x = val;
-}
-
-static inline void __ustl(__u32 val, __u32 *addr)
-{
-	struct __una_u32 *ptr = (struct __una_u32 *) addr;
-	ptr->x = val;
-}
-
-static inline void __ustw(__u16 val, __u16 *addr)
-{
-	struct __una_u16 *ptr = (struct __una_u16 *) addr;
-	ptr->x = val;
-}
-
-#define __get_unaligned(ptr, size) ({		\
-	const void *__gu_p = ptr;		\
-	__u64 val;				\
-	switch (size) {				\
-	case 1:					\
-		val = *(const __u8 *)__gu_p;	\
-		break;				\
-	case 2:					\
-		val = __uldw(__gu_p);		\
-		break;				\
-	case 4:					\
-		val = __uldl(__gu_p);		\
-		break;				\
-	case 8:					\
-		val = __uldq(__gu_p);		\
-		break;				\
-	default:				\
-		bad_unaligned_access_length();	\
-	};					\
-	(__typeof__(*(ptr)))val;		\
-})
-
-#define __put_unaligned(val, ptr, size)		\
-do {						\
-	void *__gu_p = ptr;			\
-	switch (size) {				\
-	case 1:					\
-		*(__u8 *)__gu_p = val;		\
-	        break;				\
-	case 2:					\
-		__ustw(val, __gu_p);		\
-		break;				\
-	case 4:					\
-		__ustl(val, __gu_p);		\
-		break;				\
-	case 8:					\
-		__ustq(val, __gu_p);		\
-		break;				\
-	default:				\
-	    	bad_unaligned_access_length();	\
-	};					\
-} while(0)
-
-#endif /* _UNALIGNED_H */

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfsprogs: fix unaligned access in libxfs

[Eric Sandeen]

Christoph Hellwig wrote:
> The get/put unaligned handlers we use to access the extent descriptor
> are not good enough for architectures like Sparc that do not tolerate
> dereferencing unaligned pointers.  Replace the implementation with the
> one the kernel uses on these architectures.  It might be a tad
> slower on architectures like x86, but I don't want to have multiple
> implementations around to not let the testing matrix explode.
> Also remove the unaligned.h header which includes another implementation
> for unaligned access we don't actually use anymore.
> 
> Note that the little change to xfs_inode.c needs to go into the kernel
> aswell, I will send a patch for that shortly.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reported-by: Gabriel Vlasiu <gabrielvlasiu@gmail.com>
> Tested-by: Gabriel Vlasiu <gabrielvlasiu@gmail.com>

Looks good to me.

Reviewed-by: Eric Sandeen <sandeen@sandeen.net>

> Index: xfsprogs-dev/libxfs/xfs.h
> ===================================================================
> --- xfsprogs-dev.orig/libxfs/xfs.h	2009-08-27 10:06:13.377855006 -0300
> +++ xfsprogs-dev/libxfs/xfs.h	2009-08-27 10:06:26.537884492 -0300
> @@ -127,19 +127,37 @@ static inline int __do_div(unsigned long
>  #define max_t(type,x,y) \
>  	({ type __x = (x); type __y = (y); __x > __y ? __x: __y; })
>  
> -/* only 64 bit accesses used in xfs kernel code */
> -static inline __u64 get_unaligned_be64(void *ptr)
> +
> +static inline __uint32_t __get_unaligned_be32(const __uint8_t *p)
> +{
> +        return p[0] << 24 | p[1] << 16 | p[2] << 8 | p[3];
> +}
> +
> +static inline __uint64_t get_unaligned_be64(void *p)
>  {
> -	__be64	__tmp;
> -	memmove(&__tmp, ptr, 8);
> -	return be64_to_cpu(__tmp);
> +	return (__uint64_t)__get_unaligned_be32(p) << 32 |
> +			   __get_unaligned_be32(p + 4);
>  }
>  
> -static inline void put_unaligned(__be64 val, void *ptr)
> +static inline void __put_unaligned_be16(__uint16_t val, __uint8_t *p)
>  {
> -	memmove(ptr, &val, 8);
> +	*p++ = val >> 8;
> +	*p++ = val;
>  }
>  
> +static inline void __put_unaligned_be32(__uint32_t val, __uint8_t *p)
> +{
> +	__put_unaligned_be16(val >> 16, p);
> +	__put_unaligned_be16(val, p + 2);
> +}
> +
> +static inline void put_unaligned_be64(__uint64_t val, void *p)
> +{
> +	__put_unaligned_be32(val >> 32, p);
> +	__put_unaligned_be32(val, p + 4);
> +}
> +
> +
>  static inline __attribute__((const))
>  int is_power_of_2(unsigned long n)
>  {
> Index: xfsprogs-dev/libxfs/xfs_inode.c
> ===================================================================
> --- xfsprogs-dev.orig/libxfs/xfs_inode.c	2009-08-27 10:06:13.385884867 -0300
> +++ xfsprogs-dev/libxfs/xfs_inode.c	2009-08-27 10:06:26.541855737 -0300
> @@ -1056,8 +1056,8 @@ xfs_iextents_copy(
>  		}
>  
>  		/* Translate to on disk format */
> -		put_unaligned(cpu_to_be64(ep->l0), &dp->l0);
> -		put_unaligned(cpu_to_be64(ep->l1), &dp->l1);
> +		put_unaligned_be64(ep->l0, &dp->l0);
> +		put_unaligned_be64(ep->l1, &dp->l1);
>  		dp++;
>  		copied++;
>  	}
> Index: xfsprogs-dev/include/Makefile
> ===================================================================
> --- xfsprogs-dev.orig/include/Makefile	2009-08-28 12:29:42.257922053 -0300
> +++ xfsprogs-dev/include/Makefile	2009-08-28 12:29:55.830456736 -0300
> @@ -37,7 +37,7 @@ PHFILES = darwin.h freebsd.h irix.h linu
>  DKHFILES = volume.h fstyp.h dvh.h
>  LSRCFILES = $(shell echo $(PHFILES) | sed -e "s/$(PKG_PLATFORM).h//g")
>  LSRCFILES += platform_defs.h.in builddefs.in buildmacros buildrules install-sh
> -LSRCFILES += $(DKHFILES) command.h input.h path.h project.h unaligned.h
> +LSRCFILES += $(DKHFILES) command.h input.h path.h project.h
>  LDIRT = xfs disk
>  
>  default install: xfs disk
> Index: xfsprogs-dev/include/unaligned.h
> ===================================================================
> --- xfsprogs-dev.orig/include/unaligned.h	2009-08-28 12:29:34.710448280 -0300
> +++ /dev/null	1970-01-01 00:00:00.000000000 +0000
> @@ -1,120 +0,0 @@
> -#ifndef _UNALIGNED_H_
> -#define _UNALIGNED_H_
> -
> -/*
> - * For the benefit of those who are trying to port Linux to another
> - * architecture, here are some C-language equivalents. 
> - *
> - * This is based almost entirely upon Richard Henderson's
> - * asm-alpha/unaligned.h implementation.  Some comments were
> - * taken from David Mosberger's asm-ia64/unaligned.h header.
> - */
> -
> -/* 
> - * The main single-value unaligned transfer routines.
> - */
> -#define get_unaligned(ptr) \
> -	__get_unaligned((ptr), sizeof(*(ptr)))
> -#define put_unaligned(x,ptr) \
> -	__put_unaligned((__u64)(x), (ptr), sizeof(*(ptr)))
> -
> -/*
> - * This function doesn't actually exist.  The idea is that when
> - * someone uses the macros below with an unsupported size (datatype),
> - * the linker will alert us to the problem via an unresolved reference
> - * error.
> - */
> -extern void bad_unaligned_access_length(void) __attribute__((noreturn));
> -
> -struct __una_u64 { __u64 x __attribute__((packed)); };
> -struct __una_u32 { __u32 x __attribute__((packed)); };
> -struct __una_u16 { __u16 x __attribute__((packed)); };
> -
> -/*
> - * Elemental unaligned loads 
> - */
> -
> -static inline __u64 __uldq(const __u64 *addr)
> -{
> -	const struct __una_u64 *ptr = (const struct __una_u64 *) addr;
> -	return ptr->x;
> -}
> -
> -static inline __u32 __uldl(const __u32 *addr)
> -{
> -	const struct __una_u32 *ptr = (const struct __una_u32 *) addr;
> -	return ptr->x;
> -}
> -
> -static inline __u16 __uldw(const __u16 *addr)
> -{
> -	const struct __una_u16 *ptr = (const struct __una_u16 *) addr;
> -	return ptr->x;
> -}
> -
> -/*
> - * Elemental unaligned stores 
> - */
> -
> -static inline void __ustq(__u64 val, __u64 *addr)
> -{
> -	struct __una_u64 *ptr = (struct __una_u64 *) addr;
> -	ptr->x = val;
> -}
> -
> -static inline void __ustl(__u32 val, __u32 *addr)
> -{
> -	struct __una_u32 *ptr = (struct __una_u32 *) addr;
> -	ptr->x = val;
> -}
> -
> -static inline void __ustw(__u16 val, __u16 *addr)
> -{
> -	struct __una_u16 *ptr = (struct __una_u16 *) addr;
> -	ptr->x = val;
> -}
> -
> -#define __get_unaligned(ptr, size) ({		\
> -	const void *__gu_p = ptr;		\
> -	__u64 val;				\
> -	switch (size) {				\
> -	case 1:					\
> -		val = *(const __u8 *)__gu_p;	\
> -		break;				\
> -	case 2:					\
> -		val = __uldw(__gu_p);		\
> -		break;				\
> -	case 4:					\
> -		val = __uldl(__gu_p);		\
> -		break;				\
> -	case 8:					\
> -		val = __uldq(__gu_p);		\
> -		break;				\
> -	default:				\
> -		bad_unaligned_access_length();	\
> -	};					\
> -	(__typeof__(*(ptr)))val;		\
> -})
> -
> -#define __put_unaligned(val, ptr, size)		\
> -do {						\
> -	void *__gu_p = ptr;			\
> -	switch (size) {				\
> -	case 1:					\
> -		*(__u8 *)__gu_p = val;		\
> -	        break;				\
> -	case 2:					\
> -		__ustw(val, __gu_p);		\
> -		break;				\
> -	case 4:					\
> -		__ustl(val, __gu_p);		\
> -		break;				\
> -	case 8:					\
> -		__ustq(val, __gu_p);		\
> -		break;				\
> -	default:				\
> -	    	bad_unaligned_access_length();	\
> -	};					\
> -} while(0)
> -
> -#endif /* _UNALIGNED_H */
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

pushing out a new xfsprogs release, was Re: [PATCH] xfsprogs: fix unaligned access in libxfs

[Christoph Hellwig]

On Fri, Aug 28, 2009 at 10:15:51AM -0700, Eric Sandeen wrote:
> Looks good to me.
>
> Reviewed-by: Eric Sandeen <sandeen@sandeen.net>

Thanks.  What do people thing about pushing out a 3.0.3 xfsprogs
release?

Since 3.0.1 (discounting 3.0.2 as a Debian-only thing quickly following
3.0.1): we have the following changes the list below.  It's not a whole
lot, but everything is relatively small fixes, the the unaligned thing
makes the 3.0 series so far unusable on strict alignment platforms.

If we go for it I can update the CHANGES ASAP for the release, and
ater that we can start putting in changes for a 3.1.0 release - I have
Barry's repair speedups ready for a wider audience, Eric has the
patch to make the lazy sb counters default and I can do the long
needed resync with the kernel code.


Changes from 3.0.1 to HEAD:



Christoph Hellwig (3):
      add release.sh
      Merge branch 'master' of git://oss.sgi.com/xfs/cmds/xfsprogs
      xfsprogs: fix unaligned access in libxfs

David Chinner (1):
      mkfs: allow to make larger logs

Eric Sandeen (11):
      xfsprogs: fix readline/editline for xfs_io and xfs_quota
      xfs_io: add fallocate command
      Update CHANGES file for recent commits
      xfs_repair: catch bad depth in traverse_int_dir2block
      xfs_io: fix test for fallocate on 32bit boxes
      xfs_repair: fix agcount*agblocks overflows
      xfs_metadump: agcount*agblocks overflow
      xfs_repair: clear inodes in incorrect btree format
      the freesp doesn't support "-f" so take it out of the usage().
      xfs_db: do bounds checking in frag's scanfunc_bmap
      xfs_io: actually issue 0 size writes

Felix Blyakher (2):
      Merge branch 'master' of git://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev
      3.0.1 release

Nathan Scott (2):
      Resolve minor man page warnings reported by lintian tool.
      Debian packaging updates, and bumped minor version to 3.0.2.

Nathaniel W. Turner (1):
      xfs_repair: open filesystem device exclusively

Robert Herndon (1):
      add -x flags to include/install-sh

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: pushing out a new xfsprogs release, was Re: [PATCH] xfsprogs: fix unaligned access in libxfs

[Felix Blyakher]

On Aug 28, 2009, at 1:24 PM, Christoph Hellwig wrote:

> On Fri, Aug 28, 2009 at 10:15:51AM -0700, Eric Sandeen wrote:
>> Looks good to me.
>>
>> Reviewed-by: Eric Sandeen <sandeen@sandeen.net>
>
> Thanks.  What do people thing about pushing out a 3.0.3 xfsprogs
> release?
>
> Since 3.0.1 (discounting 3.0.2 as a Debian-only thing quickly  
> following
> 3.0.1): we have the following changes the list below.  It's not a  
> whole
> lot, but everything is relatively small fixes, the the unaligned thing
> makes the 3.0 series so far unusable on strict alignment platforms.

Seems like a bunch of useful changes in xfs_repair, xfs_db and xfs_io.

Also it's passed 3 months since the last release, and it's a good idea
to keep the quarterly update.

> If we go for it I can update the CHANGES ASAP for the release,

I'll package it up right after that.

> and
> ater that we can start putting in changes for a 3.1.0 release - I have
> Barry's repair speedups ready for a wider audience, Eric has the
> patch to make the lazy sb counters default and I can do the long
> needed resync with the kernel code.

Yep, those bigger changes would be right for 3.1.0 release.

Felix

>
>
>
> Changes from 3.0.1 to HEAD:
>
>
>
> Christoph Hellwig (3):
>      add release.sh
>      Merge branch 'master' of git://oss.sgi.com/xfs/cmds/xfsprogs
>      xfsprogs: fix unaligned access in libxfs
>
> David Chinner (1):
>      mkfs: allow to make larger logs
>
> Eric Sandeen (11):
>      xfsprogs: fix readline/editline for xfs_io and xfs_quota
>      xfs_io: add fallocate command
>      Update CHANGES file for recent commits
>      xfs_repair: catch bad depth in traverse_int_dir2block
>      xfs_io: fix test for fallocate on 32bit boxes
>      xfs_repair: fix agcount*agblocks overflows
>      xfs_metadump: agcount*agblocks overflow
>      xfs_repair: clear inodes in incorrect btree format
>      the freesp doesn't support "-f" so take it out of the usage().
>      xfs_db: do bounds checking in frag's scanfunc_bmap
>      xfs_io: actually issue 0 size writes
>
> Felix Blyakher (2):
>      Merge branch 'master' of git://git.kernel.org/pub/scm/fs/xfs/ 
> xfsprogs-dev
>      3.0.1 release
>
> Nathan Scott (2):
>      Resolve minor man page warnings reported by lintian tool.
>      Debian packaging updates, and bumped minor version to 3.0.2.
>
> Nathaniel W. Turner (1):
>      xfs_repair: open filesystem device exclusively
>
> Robert Herndon (1):
>      add -x flags to include/install-sh
>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

RE: [PATCH] xfsprogs: fix unaligned access in libxfs

[Alex Elder]

Christoph Hellwig wrote:
> The get/put unaligned handlers we use to access the extent descriptor
> are not good enough for architectures like Sparc that do not tolerate
> dereferencing unaligned pointers.  Replace the implementation with the
> one the kernel uses on these architectures.  It might be a tad
> slower on architectures like x86, but I don't want to have multiple
> implementations around to not let the testing matrix explode.
> Also remove the unaligned.h header which includes another implementation
> for unaligned access we don't actually use anymore.
> 
> Note that the little change to xfs_inode.c needs to go into the kernel
> aswell, I will send a patch for that shortly.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reported-by: Gabriel Vlasiu <gabrielvlasiu@gmail.com>
> Tested-by: Gabriel Vlasiu <gabrielvlasiu@gmail.com>

Looks good.  Note comment about interface below.

Reviewed-by: Alex Elder <aelder@sgi.com>


> Index: xfsprogs-dev/libxfs/xfs.h
> ===================================================================
> --- xfsprogs-dev.orig/libxfs/xfs.h	2009-08-27 10:06:13.377855006 -0300
> +++ xfsprogs-dev/libxfs/xfs.h	2009-08-27 10:06:26.537884492 -0300
> @@ -127,19 +127,37 @@ static inline int __do_div(unsigned long
>  #define max_t(type,x,y) \
>  	({ type __x = (x); type __y = (y); __x > __y ? __x: __y; })
> 
> -/* only 64 bit accesses used in xfs kernel code */
> -static inline __u64 get_unaligned_be64(void *ptr)
> +
> +static inline __uint32_t __get_unaligned_be32(const __uint8_t *p)


It would be nice for this interface (and others) to take a (void *)
argument, but that would also mean immediately casting it to do
pointer arithmetic portably.  Not a big deal.

> +{
> +        return p[0] << 24 | p[1] << 16 | p[2] << 8 | p[3];
> +}
> +
> +static inline __uint64_t get_unaligned_be64(void *p)
>  {

. . .

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfsprogs: fix unaligned access in libxfs

[Christoph Hellwig]

On Fri, Aug 28, 2009 at 04:39:49PM -0500, Alex Elder wrote:
> > +static inline __uint32_t __get_unaligned_be32(const __uint8_t *p)
> 
> 
> It would be nice for this interface (and others) to take a (void *)
> argument, but that would also mean immediately casting it to do
> pointer arithmetic portably.  Not a big deal.

the __ routines are internal implementation details, the kernel
has non-__ prefixed versions that take void pointers, but I didn't
bother to add them to xfsprogs as we only do unaligned access to
64bit values.

That beeing said I dislike the interface taking void pointers as
it removes important error checking.  The actually visible interface
should take __be16/32/64 types to enforce they are used on the right
type.  Once I get some time I will push for that in the kernel again,
but for xfsprogs I really do want to stick to the kernel interfaces so
that we can share the code unmodified.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

RE: [PATCH] xfsprogs: fix unaligned access in libxfs

[Alex Elder]

Christoph Hellwig wrote:
> On Fri, Aug 28, 2009 at 04:39:49PM -0500, Alex Elder wrote:
>>> +static inline __uint32_t __get_unaligned_be32(const __uint8_t *p)
>> 
>> 
>> It would be nice for this interface (and others) to take a (void *)
>> argument, but that would also mean immediately casting it to do
>> pointer arithmetic portably.  Not a big deal.
> 
> the __ routines are internal implementation details, the kernel
> has non-__ prefixed versions that take void pointers, but I didn't
> bother to add them to xfsprogs as we only do unaligned access to
> 64bit values.
> 
> That beeing said I dislike the interface taking void pointers as
> it removes important error checking.  The actually visible interface
> should take __be16/32/64 types to enforce they are used on the right
> type.  Once I get some time I will push for that in the kernel again,
> but for xfsprogs I really do want to stick to the kernel interfaces so
> that we can share the code unmodified.

The results should be int types:  __be16, etc., I agree.  But
I'm saying the unaligned addresses--which we (now) make no
assumptions about and treat as character pointers--I think
the void pointer explicitly captures that unaligned quality
(and avoids any need for a cast).

In any event I think it's fine either way.  Really not a big
deal, just a tiny detail that only a geek could love to
argue about.

					-Alex

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: pushing out a new xfsprogs release, was Re: [PATCH] xfsprogs: fix unaligned access in libxfs

[Michael Monnerie]

On Freitag 28 August 2009 Christoph Hellwig wrote:
> What do people thing about pushing out a 3.0.3 xfsprogs
> release?

Is there the patch to repair my filesystem? I sent the metadump to Eric 
once, I think he did some patches. Are they there, or do I need 
something else also? I'm still having the problem, and would like to fix 
it.

mfg zmi
-- 
// Michael Monnerie, Ing.BSc    -----      http://it-management.at
// Tel: 0660 / 415 65 31                      .network.your.ideas.
// PGP Key:         "curl -s http://zmi.at/zmi.asc | gpg --import"
// Fingerprint: AC19 F9D5 36ED CD8A EF38  500E CE14 91F7 1C12 09B4
// Keyserver: wwwkeys.eu.pgp.net                  Key-ID: 1C1209B4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: pushing out a new xfsprogs release, was Re: [PATCH] xfsprogs: fix unaligned access in libxfs

[Eric Sandeen]

Michael Monnerie wrote:
> On Freitag 28 August 2009 Christoph Hellwig wrote:
>> What do people thing about pushing out a 3.0.3 xfsprogs
>> release?
> 
> Is there the patch to repair my filesystem? I sent the metadump to Eric 
> once, I think he did some patches. Are they there, or do I need 
> something else also? I'm still having the problem, and would like to fix 
> it.
> 
> mfg zmi

Afraid I haven't fixed that one yet, sorry.  If you could open a bug on 
xfs.org's bugzilla it'd be much better for tracking ...

I did find that building xfsprogs w/o debug ASSERTs turned on, I could 
get a clean fs after 2 runs, just to get you going again...

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: pushing out a new xfsprogs release, was Re: [PATCH] xfsprogs: fix unaligned access in libxfs

[Michael Monnerie]

On Sonntag 30 August 2009 Eric Sandeen wrote:
> Afraid I haven't fixed that one yet, sorry.  If you could open a bug
> on xfs.org's bugzilla it'd be much better for tracking ...

OK, looked there, but there are only 2 links: to kernel.org or 
oss.sgi.com bugzillas. Which one should I use?
BTW, wouldn't it be better to declare on the Wiki which bugzilla to use?

> I did find that building xfsprogs w/o debug ASSERTs turned on, I
> could get a clean fs after 2 runs, just to get you going again...

What would I need to take out exactly? Sorry, I don't know what "debug 
ASSERTs" are. I've seen from patches sent here those are commands, but 
which ones are for debug?

mfg zmi
-- 
// Michael Monnerie, Ing.BSc    -----      http://it-management.at
// Tel: 0660 / 415 65 31                      .network.your.ideas.
// PGP Key:         "curl -s http://zmi.at/zmi.asc | gpg --import"
// Fingerprint: AC19 F9D5 36ED CD8A EF38  500E CE14 91F7 1C12 09B4
// Keyserver: wwwkeys.eu.pgp.net                  Key-ID: 1C1209B4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs