[PATCH 14/14] PMFS: Fix endian bugs in journal.c

Matthew Wilcox matthew.r.wilcox at intel.com
Mon Oct 7 09:38:04 EDT 2013


From: Matthew Wilcox <willy at linux.intel.com>

The code in pmfs_clean_journal() assumed that the CPU was little-endian
(ie that reading a 64-bit quantity and then masking it was 2^31-1 would
return the data in the lower 32-bits of storage).  Make that assumption
true by swapping the full 64-bit quantity read, and then shifting/masking
to extract the various components of it.

Fixing the code which writes this location is even more interesting
because we want to write the 64-bit quantity atomically, using
set_64bit().  Unfortunately, set_64bit() does not take a __le64 but a
u64, so first we calculate the __le64 value we want, then we throw away
all the type checking by using __force.  Not my favourite way to fix
the problem, but I didn't want to add a set_64bit_le or similar.

Signed-off-by: Matthew Wilcox <matthew.r.wilcox at intel.com>
---
 fs/pmfs/journal.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/fs/pmfs/journal.c b/fs/pmfs/journal.c
index 98c54f4..64ffcd1 100644
--- a/fs/pmfs/journal.c
+++ b/fs/pmfs/journal.c
@@ -320,7 +320,7 @@ static void pmfs_clean_journal(struct super_block *sb, bool unmount)
 	uint32_t head = le32_to_cpu(journal->head);
 	uint32_t new_head, tail;
 	uint16_t gen_id;
-	volatile u64 *ptr_tail_genid = (volatile u64 *)&journal->tail;
+	volatile __le64 *ptr_tail_genid = (volatile __le64 *)&journal->tail;
 	u64 tail_genid;
 	pmfs_logentry_t *le;
 
@@ -329,9 +329,9 @@ static void pmfs_clean_journal(struct super_block *sb, bool unmount)
 	 * to write to journal's tail and gen_id atomically, we thought we
 	 * should use volatile to read them simultaneously and avoid locking
 	 * them. */
-	tail_genid = *ptr_tail_genid;
-	tail = le32_to_cpu(tail_genid & 0xFFFFFFFF);
-	gen_id = le16_to_cpu((tail_genid >> 32) & 0xFFFF);
+	tail_genid = le64_to_cpu(*ptr_tail_genid);
+	tail = tail_genid & 0xFFFFFFFF;
+	gen_id = (tail_genid >> 32) & 0xFFFF;
 
 	/* journal wraparound happened. so head points to prev generation id */
 	if (tail < head)
@@ -539,16 +539,12 @@ again:
 	 * that we don't have any wraparound within a transaction */
 	pmfs_memunlock_range(sb, journal, sizeof(*journal));
 	if (tail >= sbi->jsize) {
-		volatile u64 *ptr;
+		u64 *ptr;
 		tail = 0;
-		/* write the gen_id and tail atomically. Use of volatile is
-		 * normally prohibited in kernel code, but it is required here
-		 * because we want to write atomically against power failures
-		 * and locking can't provide that. */
-		ptr = (volatile u64 *)&journal->tail;
+		ptr = (u64 *)&journal->tail;
 		/* writing 8-bytes atomically setting tail to 0 */
-		set_64bit(ptr, (u64)cpu_to_le16(next_gen_id(le16_to_cpu(
-				journal->gen_id))) << 32);
+		set_64bit(ptr, (__force u64)cpu_to_le64((u64)next_gen_id(
+					le16_to_cpu(journal->gen_id)) << 32));
 		pmfs_memlock_range(sb, journal, sizeof(*journal));
 		pmfs_dbg_trans("journal wrapped. tail %x gid %d cur tid %d\n",
 			le32_to_cpu(journal->tail),le16_to_cpu(journal->gen_id),
-- 
1.8.4.rc3




More information about the Linux-pmfs mailing list