[PATCH] xfsprogs: fix unaligned access in libxfs
Eric Sandeen
sandeen at sandeen.net
Fri Aug 28 12:15:51 CDT 2009
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 at lst.de>
> Reported-by: Gabriel Vlasiu <gabrielvlasiu at gmail.com>
> Tested-by: Gabriel Vlasiu <gabrielvlasiu at gmail.com>
Looks good to me.
Reviewed-by: Eric Sandeen <sandeen at 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 at oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
>
More information about the xfs
mailing list