xfs
[Top] [All Lists]

Re: [PATCH] fix dir2 shortform structures on ARM old ABI

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH] fix dir2 shortform structures on ARM old ABI
From: "Josef 'Jeff' Sipek" <jeffpc@xxxxxxxxxxxxxx>
Date: Sat, 15 Mar 2008 00:17:22 -0400
Cc: xfs-oss <xfs@xxxxxxxxxxx>, patches@xxxxxxxxxxxxxxxx
In-reply-to: <47DB4181.7040603@xxxxxxxxxxx>
References: <47DB4181.7040603@xxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.16 (2007-06-11)
On Fri, Mar 14, 2008 at 10:24:49PM -0500, Eric Sandeen wrote:
> This should fix the longstanding issues with xfs and old ABI
> arm boxes, which lead to various asserts and xfs shutdowns,
> and for which an (incorrect) patch has been floating around
> for years.  (Said patch made ARM internally consistent, but
> altered the normal xfs on-disk format such that it looked
> corrupted on other architectures):
> http://lists.arm.linux.org.uk/lurker/message/20040311.002034.5ecf21a2.html
...
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxxx>
> 
> ---
> 
> Index: linux-2.6.24/fs/xfs/linux-2.6/xfs_linux.h
> ===================================================================
> --- linux-2.6.24.orig/fs/xfs/linux-2.6/xfs_linux.h
> +++ linux-2.6.24/fs/xfs/linux-2.6/xfs_linux.h
> @@ -300,4 +300,11 @@ static inline __uint64_t howmany_64(__ui
>       return x;
>  }
>  
> +/* ARM old ABI has some weird alignment/padding */
> +#if defined(__arm__) && !defined(__ARM_EABI__)
> +#define __arch_pack __attribute__((packed))
> +#else
> +#define __arch_pack
> +#endif

Shouldn't this be unconditional? Just because it ends up being ok on x86
doesn't mean that it won't break some time later on...(do we want another
bad_features2 incident?)

> +
>  #endif /* __XFS_LINUX__ */
> Index: linux-2.6.24/fs/xfs/xfs_dir2_sf.h
> ===================================================================
> --- linux-2.6.24.orig/fs/xfs/xfs_dir2_sf.h
> +++ linux-2.6.24/fs/xfs/xfs_dir2_sf.h
> @@ -62,7 +62,7 @@ typedef union {
>   * Normalized offset (in a data block) of the entry, really 
> xfs_dir2_data_off_t.
>   * Only need 16 bits, this is the byte offset into the single block form.
>   */
> -typedef struct { __uint8_t i[2]; } xfs_dir2_sf_off_t;
> +typedef struct { __uint8_t i[2]; } __arch_pack xfs_dir2_sf_off_t;
>  
>  /*
>   * The parent directory has a dedicated field, and the self-pointer must
> @@ -76,14 +76,14 @@ typedef struct xfs_dir2_sf_hdr {
>       __uint8_t               count;          /* count of entries */
>       __uint8_t               i8count;        /* count of 8-byte inode #s */
>       xfs_dir2_inou_t         parent;         /* parent dir inode number */
> -} xfs_dir2_sf_hdr_t;
> +} __arch_pack xfs_dir2_sf_hdr_t;
>  
>  typedef struct xfs_dir2_sf_entry {
>       __uint8_t               namelen;        /* actual name length */
>       xfs_dir2_sf_off_t       offset;         /* saved offset */
>       __uint8_t               name[1];        /* name, variable size */
>       xfs_dir2_inou_t         inumber;        /* inode number, var. offset */
> -} xfs_dir2_sf_entry_t;
> +} __arch_pack xfs_dir2_sf_entry_t; 
>  
>  typedef struct xfs_dir2_sf {
>       xfs_dir2_sf_hdr_t       hdr;            /* shortform header */

A very simple patch! I like it (minus the condition vs. unconditional
packing - see above).

Josef 'Jeff' Sipek.

-- 
Penguin : Linux version 2.6.23.1 on an i386 machine (6135.23 BogoMips).


<Prev in Thread] Current Thread [Next in Thread>