xfs
[Top] [All Lists]

[PATCH] fix dir2 shortform structures on ARM old ABI

To: xfs-oss <xfs@xxxxxxxxxxx>
Subject: [PATCH] fix dir2 shortform structures on ARM old ABI
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Fri, 14 Mar 2008 22:24:49 -0500
Cc: patches@xxxxxxxxxxxxxxxx
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.12 (Macintosh/20080213)
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

Old ABI ARM has interesting packing & padding; for example
on ARM old abi:

struct xfs_dir2_sf_entry {
        __uint8_t                  namelen;      /*     0     1 */

        /* XXX 3 bytes hole, try to pack */

        xfs_dir2_sf_off_t          offset;       /*     4     4 */
        __uint8_t                  name[1];      /*     8     1 */

        /* XXX 3 bytes hole, try to pack */

        xfs_dir2_inou_t            inumber;      /*    12     8 */

        /* size: 20, cachelines: 1 */
        /* sum members: 14, holes: 2, sum holes: 6 */
        /* last cacheline: 20 bytes */
};

but on x86:

struct xfs_dir2_sf_entry {
        __uint8_t                  namelen;      /*     0     1 */
        xfs_dir2_sf_off_t          offset;       /*     1     2 */
        __uint8_t                  name[1];      /*     3     1 */
        xfs_dir2_inou_t            inumber;      /*     4     8 */

        /* size: 12, cachelines: 1 */
        /* last cacheline: 12 bytes */
};

... this sort of discrepancy leads to problems.

I've verified this patch by comparing the on-disk structure 
layouts using pahole from the dwarves package, as well as 
running through a bit of xfsqa under qemu-arm, modified so 
that the check/repair phase after each test actually executes 
check/repair from the x86 host, on the filesystem populated 
by the arm emulator.  Thus far it all looks good.

There are 2 other structures with extra padding at the end, 
but they don't seem to cause trouble.  I suppose they could 
be packed as well: xfs_dir2_data_unused_t and xfs_dir2_sf_t.

Note that userspace needs a similar treatment, and any
filesystems which were running with the previous rogue
"fix" will now see corruption (either in the kernel, or
during xfs_repair) with this fix properly in place; it 
may be worth teaching xfs_repair to identify and fix that 
specific issue.

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
+
 #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 */



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