All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] ext4: fix usless declarations
@ 2013-04-03 10:58 Dmitry Monakhov
  2013-04-03 10:58 ` [PATCH 2/2] ext4: fix cpu_vs_disk conversions Dmitry Monakhov
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Dmitry Monakhov @ 2013-04-03 10:58 UTC (permalink / raw)
  To: linux-ext4; +Cc: tytso, wenqing.lz, Dmitry Monakhov

This patch should fix sparse complains about shadow declatations.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/ialloc.c      |    1 -
 fs/ext4/ioctl.c       |    1 -
 fs/ext4/mballoc.c     |    1 -
 fs/ext4/move_extent.c |    1 -
 4 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 6c5bb8d..4c358f7 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -899,7 +899,6 @@ got:
 	if (EXT4_HAS_RO_COMPAT_FEATURE(sb,
 			EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) {
 		__u32 csum;
-		struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
 		__le32 inum = cpu_to_le32(inode->i_ino);
 		__le32 gen = cpu_to_le32(inode->i_generation);
 		csum = ext4_chksum(sbi, sbi->s_csum_seed, (__u8 *)&inum,
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index a07b7bc..fb4e77f 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -355,7 +355,6 @@ group_add_out:
 
 	case EXT4_IOC_RESIZE_FS: {
 		ext4_fsblk_t n_blocks_count;
-		struct super_block *sb = inode->i_sb;
 		int err = 0, err2 = 0;
 		ext4_group_t o_group = EXT4_SB(sb)->s_groups_count;
 
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 580aada..5bf8056 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -860,7 +860,6 @@ static int ext4_mb_init_cache(struct page *page, char *incore)
 
 	first_block = page->index * blocks_per_page;
 	for (i = 0; i < blocks_per_page; i++) {
-		int group;
 
 		group = (first_block + i) >> 1;
 		if (group >= ngroups)
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 33e1c08..7f3ed63 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -856,7 +856,6 @@ mext_page_mkuptodate(struct page *page, unsigned from, unsigned to)
 		if (buffer_uptodate(bh))
 			continue;
 		if (!buffer_mapped(bh)) {
-			int err = 0;
 			err = ext4_get_block(inode, block, bh, 0);
 			if (err) {
 				SetPageError(page);
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] ext4: fix cpu_vs_disk conversions
  2013-04-03 10:58 [PATCH 1/2] ext4: fix usless declarations Dmitry Monakhov
@ 2013-04-03 10:58 ` Dmitry Monakhov
  2013-04-03 12:33   ` Theodore Ts'o
                     ` (3 more replies)
  2013-04-03 12:26 ` [PATCH 1/2] ext4: fix usless declarations Theodore Ts'o
  2013-04-10  2:50 ` [1/2] " Theodore Ts'o
  2 siblings, 4 replies; 16+ messages in thread
From: Dmitry Monakhov @ 2013-04-03 10:58 UTC (permalink / raw)
  To: linux-ext4; +Cc: tytso, wenqing.lz, Dmitry Monakhov

All new features are broken on bigendian hosts due to lack of conversion:
es_cache: https://lkml.org/lkml/2013/3/28/64
inode's csum and ext_to_ind_migrate are also broken.

Testcase: make C=2 CF="-D__CHECK_ENDIAN__

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/extents.c           |   12 ++++++------
 fs/ext4/indirect.c          |    4 ++--
 fs/ext4/inode.c             |    8 ++++----
 fs/ext4/mmp.c               |    2 +-
 fs/ext4/namei.c             |    4 ++--
 fs/ext4/super.c             |    4 ++--
 fs/ext4/xattr.c             |   11 +++++------
 include/trace/events/ext4.h |    6 +++---
 8 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 1530cf4..3c3410e 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2784,7 +2784,7 @@ again:
 	}
 
 	trace_ext4_ext_remove_space_done(inode, start, depth, partial_cluster,
-			path->p_hdr->eh_entries);
+					 path->p_hdr->eh_entries);
 
 	/* If we still have something in the partial cluster and we have removed
 	 * even the first extent, then we should free the blocks in the partial
@@ -2999,20 +2999,20 @@ static int ext4_split_extent_at(handle_t *handle,
 			if (split_flag & EXT4_EXT_DATA_VALID1) {
 				err = ext4_ext_zeroout(inode, ex2);
 				zero_ex.ee_block = ex2->ee_block;
-				zero_ex.ee_len = ext4_ext_get_actual_len(ex2);
+				zero_ex.ee_len = cpu_to_le16(ext4_ext_get_actual_len(ex2));
 				ext4_ext_store_pblock(&zero_ex,
 						      ext4_ext_pblock(ex2));
 			} else {
 				err = ext4_ext_zeroout(inode, ex);
 				zero_ex.ee_block = ex->ee_block;
-				zero_ex.ee_len = ext4_ext_get_actual_len(ex);
+				zero_ex.ee_len = cpu_to_le16(ext4_ext_get_actual_len(ex));
 				ext4_ext_store_pblock(&zero_ex,
 						      ext4_ext_pblock(ex));
 			}
 		} else {
 			err = ext4_ext_zeroout(inode, &orig_ex);
 			zero_ex.ee_block = orig_ex.ee_block;
-			zero_ex.ee_len = ext4_ext_get_actual_len(&orig_ex);
+			zero_ex.ee_len = cpu_to_le16(ext4_ext_get_actual_len(&orig_ex));
 			ext4_ext_store_pblock(&zero_ex,
 					      ext4_ext_pblock(&orig_ex));
 		}
@@ -3272,7 +3272,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 		if (err)
 			goto out;
 		zero_ex.ee_block = ex->ee_block;
-		zero_ex.ee_len = ext4_ext_get_actual_len(ex);
+		zero_ex.ee_len = cpu_to_le16(ext4_ext_get_actual_len(ex));
 		ext4_ext_store_pblock(&zero_ex, ext4_ext_pblock(ex));
 
 		err = ext4_ext_get_access(handle, inode, path + depth);
@@ -4639,7 +4639,7 @@ int ext4_ind_migrate(struct inode *inode)
 	eh = ext_inode_hdr(inode);
 	ex  = EXT_FIRST_EXTENT(eh);
 	if (ext4_blocks_count(es) > EXT4_MAX_BLOCK_FILE_PHYS ||
-	    eh->eh_depth != 0 || eh->eh_entries > 1) {
+	    eh->eh_depth != 0 || le16_to_cpu(eh->eh_entries) > 1) {
 		ret = -EOPNOTSUPP;
 		goto errout;
 	}
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 80a798b..98be6f6 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -1324,9 +1324,9 @@ static int free_hole_blocks(handle_t *handle, struct inode *inode,
 		blk = *i_data;
 		if (level > 0) {
 			ext4_lblk_t first2;
-			bh = sb_bread(inode->i_sb, blk);
+			bh = sb_bread(inode->i_sb, le32_to_cpu(blk));
 			if (!bh) {
-				EXT4_ERROR_INODE_BLOCK(inode, blk,
+				EXT4_ERROR_INODE_BLOCK(inode, le32_to_cpu(blk),
 						       "Read failure");
 				return -EIO;
 			}
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 7712aff..a2c4f14 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -55,21 +55,21 @@ static __u32 ext4_inode_csum(struct inode *inode, struct ext4_inode *raw,
 	__u16 csum_hi = 0;
 	__u32 csum;
 
-	csum_lo = raw->i_checksum_lo;
+	csum_lo = le16_to_cpu(raw->i_checksum_lo);
 	raw->i_checksum_lo = 0;
 	if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE &&
 	    EXT4_FITS_IN_INODE(raw, ei, i_checksum_hi)) {
-		csum_hi = raw->i_checksum_hi;
+		csum_hi = le16_to_cpu(raw->i_checksum_hi);
 		raw->i_checksum_hi = 0;
 	}
 
 	csum = ext4_chksum(sbi, ei->i_csum_seed, (__u8 *)raw,
 			   EXT4_INODE_SIZE(inode->i_sb));
 
-	raw->i_checksum_lo = csum_lo;
+	raw->i_checksum_lo = cpu_to_le16(csum_lo);
 	if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE &&
 	    EXT4_FITS_IN_INODE(raw, ei, i_checksum_hi))
-		raw->i_checksum_hi = csum_hi;
+		raw->i_checksum_hi = cpu_to_le16(csum_hi);
 
 	return csum;
 }
diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index f9b5515..b3b1f7d 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -7,7 +7,7 @@
 #include "ext4.h"
 
 /* Checksumming functions */
-static __u32 ext4_mmp_csum(struct super_block *sb, struct mmp_struct *mmp)
+static __le32 ext4_mmp_csum(struct super_block *sb, struct mmp_struct *mmp)
 {
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	int offset = offsetof(struct mmp_struct, mmp_checksum);
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 3825d6a..5a29ea6 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -420,11 +420,11 @@ static __le32 ext4_dx_csum(struct inode *inode, struct ext4_dir_entry *dirent,
 	int size;
 
 	size = count_offset + (count * sizeof(struct dx_entry));
-	old_csum = t->dt_checksum;
+	old_csum = le32_to_cpu(t->dt_checksum);
 	t->dt_checksum = 0;
 	csum = ext4_chksum(sbi, ei->i_csum_seed, (__u8 *)dirent, size);
 	csum = ext4_chksum(sbi, csum, (__u8 *)t, sizeof(struct dx_tail));
-	t->dt_checksum = old_csum;
+	t->dt_checksum = cpu_to_le32(old_csum);
 
 	return cpu_to_le32(csum);
 }
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 280a918..c712c8f 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1954,13 +1954,13 @@ static __le16 ext4_group_desc_csum(struct ext4_sb_info *sbi, __u32 block_group,
 		__u16 old_csum;
 		__u32 csum32;
 
-		old_csum = gdp->bg_checksum;
+		old_csum = le16_to_cpu(gdp->bg_checksum);
 		gdp->bg_checksum = 0;
 		csum32 = ext4_chksum(sbi, sbi->s_csum_seed, (__u8 *)&le_group,
 				     sizeof(le_group));
 		csum32 = ext4_chksum(sbi, csum32, (__u8 *)gdp,
 				     sbi->s_desc_size);
-		gdp->bg_checksum = old_csum;
+		gdp->bg_checksum = cpu_to_le16(old_csum);
 
 		crc = csum32 & 0xFFFF;
 		goto out;
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 3a120b2..34da740 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -123,16 +123,15 @@ static __le32 ext4_xattr_block_csum(struct inode *inode,
 {
 	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
 	__u32 csum, old;
-
-	old = hdr->h_checksum;
+	__le64 dsk_block_nr = cpu_to_le64(block_nr);
+	old = le32_to_cpu(hdr->h_checksum);
 	hdr->h_checksum = 0;
-	block_nr = cpu_to_le64(block_nr);
-	csum = ext4_chksum(sbi, sbi->s_csum_seed, (__u8 *)&block_nr,
-			   sizeof(block_nr));
+	csum = ext4_chksum(sbi, sbi->s_csum_seed, (__u8 *)&dsk_block_nr,
+			   sizeof(dsk_block_nr));
 	csum = ext4_chksum(sbi, csum, (__u8 *)hdr,
 			   EXT4_BLOCK_SIZE(inode->i_sb));
 
-	hdr->h_checksum = old;
+	hdr->h_checksum = cpu_to_le32(old);
 	return cpu_to_le32(csum);
 }
 
diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
index 58459b7..d0e6864 100644
--- a/include/trace/events/ext4.h
+++ b/include/trace/events/ext4.h
@@ -1948,7 +1948,7 @@ TRACE_EVENT(ext4_remove_blocks,
 		__entry->to		= to;
 		__entry->partial	= partial_cluster;
 		__entry->ee_pblk	= ext4_ext_pblock(ex);
-		__entry->ee_lblk	= cpu_to_le32(ex->ee_block);
+		__entry->ee_lblk	= le32_to_cpu(ex->ee_block);
 		__entry->ee_len		= ext4_ext_get_actual_len(ex);
 	),
 
@@ -2052,7 +2052,7 @@ TRACE_EVENT(ext4_ext_remove_space,
 
 TRACE_EVENT(ext4_ext_remove_space_done,
 	TP_PROTO(struct inode *inode, ext4_lblk_t start, int depth,
-		ext4_lblk_t partial, unsigned short eh_entries),
+		ext4_lblk_t partial, __le16 eh_entries),
 
 	TP_ARGS(inode, start, depth, partial, eh_entries),
 
@@ -2071,7 +2071,7 @@ TRACE_EVENT(ext4_ext_remove_space_done,
 		__entry->start		= start;
 		__entry->depth		= depth;
 		__entry->partial	= partial;
-		__entry->eh_entries	= eh_entries;
+		__entry->eh_entries	= le16_to_cpu(eh_entries);
 	),
 
 	TP_printk("dev %d,%d ino %lu since %u depth %d partial %u "
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] ext4: fix usless declarations
  2013-04-03 10:58 [PATCH 1/2] ext4: fix usless declarations Dmitry Monakhov
  2013-04-03 10:58 ` [PATCH 2/2] ext4: fix cpu_vs_disk conversions Dmitry Monakhov
@ 2013-04-03 12:26 ` Theodore Ts'o
  2013-04-03 12:31   ` Dmitry Monakhov
  2013-04-10  2:50 ` [1/2] " Theodore Ts'o
  2 siblings, 1 reply; 16+ messages in thread
From: Theodore Ts'o @ 2013-04-03 12:26 UTC (permalink / raw)
  To: Dmitry Monakhov; +Cc: linux-ext4, wenqing.lz

On Wed, Apr 03, 2013 at 02:58:30PM +0400, Dmitry Monakhov wrote:
> This patch should fix sparse complains about shadow declatations.

Nothing here is fixing serious regressions, right?  This is just to
make static code checkers happy?

						- Ted

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] ext4: fix usless declarations
  2013-04-03 12:26 ` [PATCH 1/2] ext4: fix usless declarations Theodore Ts'o
@ 2013-04-03 12:31   ` Dmitry Monakhov
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Monakhov @ 2013-04-03 12:31 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: linux-ext4, wenqing.lz

On Wed, 3 Apr 2013 08:26:41 -0400, Theodore Ts'o <tytso@mit.edu> wrote:
> On Wed, Apr 03, 2013 at 02:58:30PM +0400, Dmitry Monakhov wrote:
> > This patch should fix sparse complains about shadow declatations.
> 
> Nothing here is fixing serious regressions, right?  This is just to
> make static code checkers happy?
Yep... but fixes are also quite simple.
> 
> 						- Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ext4: fix cpu_vs_disk conversions
  2013-04-03 10:58 ` [PATCH 2/2] ext4: fix cpu_vs_disk conversions Dmitry Monakhov
@ 2013-04-03 12:33   ` Theodore Ts'o
  2013-04-03 12:58     ` Dmitry Monakhov
  2013-04-03 13:04     ` Zheng Liu
  2013-04-03 15:37   ` Theodore Ts'o
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 16+ messages in thread
From: Theodore Ts'o @ 2013-04-03 12:33 UTC (permalink / raw)
  To: Dmitry Monakhov; +Cc: linux-ext4, wenqing.lz

On Wed, Apr 03, 2013 at 02:58:31PM +0400, Dmitry Monakhov wrote:
> All new features are broken on bigendian hosts due to lack of conversion:
> es_cache: https://lkml.org/lkml/2013/3/28/64
> inode's csum and ext_to_ind_migrate are also broken.
> 
> Testcase: make C=2 CF="-D__CHECK_ENDIAN__
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>

Thanks for this comprehensive patch.  I'm currently trying to decide
how much of this I should try to push in before the next 3.9-rcX
window, and how much can wait until the next merge window.

We definitely want to fix the fs corruption bug for big-endian
systems.  For the rest, I'm on the fence, since they are less likely
to bite people hard --- metadata checksum is still pretty new and not
fully supported, and the punch hole bug is again also pretty new
functionality.

Also, pushing something to Linus now could potentially disrupt the
patches in the ext4 dev tree for the next merge window.  For the zero
extents problem, there's no question what are priorities should be;
user dataloss trumps developer convenience any day.  For the rest,
what do you all think?  Are they likely to hit people hard enough that
it's worth trying to get this to Linus sooner, as opposed to having
the fixes for the rest of the big endian issues land in 3.10 and
3.9.1?

					- Ted

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ext4: fix cpu_vs_disk conversions
  2013-04-03 12:33   ` Theodore Ts'o
@ 2013-04-03 12:58     ` Dmitry Monakhov
  2013-04-03 13:04     ` Zheng Liu
  1 sibling, 0 replies; 16+ messages in thread
From: Dmitry Monakhov @ 2013-04-03 12:58 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: linux-ext4, wenqing.lz

On Wed, 3 Apr 2013 08:33:17 -0400, Theodore Ts'o <tytso@mit.edu> wrote:
> On Wed, Apr 03, 2013 at 02:58:31PM +0400, Dmitry Monakhov wrote:
> > All new features are broken on bigendian hosts due to lack of conversion:
> > es_cache: https://lkml.org/lkml/2013/3/28/64
> > inode's csum and ext_to_ind_migrate are also broken.
> > 
> > Testcase: make C=2 CF="-D__CHECK_ENDIAN__
> > 
> > Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> 
> Thanks for this comprehensive patch.  I'm currently trying to decide
> how much of this I should try to push in before the next 3.9-rcX
> window, and how much can wait until the next merge window.
> 
> We definitely want to fix the fs corruption bug for big-endian
> systems.  For the rest, I'm on the fence, since they are less likely
> to bite people hard --- metadata checksum is still pretty new and not
> fully supported, and the punch hole bug is again also pretty new
> functionality.
> 
> Also, pushing something to Linus now could potentially disrupt the
> patches in the ext4 dev tree for the next merge window.  For the zero
But how? the patch is simple as potato.
It does not conflict with any other patches in dev branch (actually i've
prepared it against ext4/dev branch)
Personally I'll vote for rebase ext4/dev to 3.9-rc5 (or rc6) 
because rc4 has a bug which prevent two of my boxes from boot.
So I can't see any difficulties with it.
But off course if we think in terms of ultra-conservative terms it is reasonable
to just push es_cache fixes to linus and leave others for devel branch.
IMHO we already have done too many mistakes during that release so it is too
late to pretend to being too cautious.
> extents problem, there's no question what are priorities should be;
> user dataloss trumps developer convenience any day.  For the rest,
> what do you all think?  Are they likely to hit people hard enough that
> it's worth trying to get this to Linus sooner, as opposed to having
> the fixes for the rest of the big endian issues land in 3.10 and
> 3.9.1?
> 
> 					- Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ext4: fix cpu_vs_disk conversions
  2013-04-03 12:33   ` Theodore Ts'o
  2013-04-03 12:58     ` Dmitry Monakhov
@ 2013-04-03 13:04     ` Zheng Liu
  2013-04-03 16:01       ` Theodore Ts'o
  1 sibling, 1 reply; 16+ messages in thread
From: Zheng Liu @ 2013-04-03 13:04 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Dmitry Monakhov, linux-ext4, wenqing.lz

On Wed, Apr 03, 2013 at 08:33:17AM -0400, Theodore Ts'o wrote:
> On Wed, Apr 03, 2013 at 02:58:31PM +0400, Dmitry Monakhov wrote:
> > All new features are broken on bigendian hosts due to lack of conversion:
> > es_cache: https://lkml.org/lkml/2013/3/28/64
> > inode's csum and ext_to_ind_migrate are also broken.
> > 
> > Testcase: make C=2 CF="-D__CHECK_ENDIAN__
> > 
> > Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> 
> Thanks for this comprehensive patch.  I'm currently trying to decide
> how much of this I should try to push in before the next 3.9-rcX
> window, and how much can wait until the next merge window.
> 
> We definitely want to fix the fs corruption bug for big-endian
> systems.  For the rest, I'm on the fence, since they are less likely
> to bite people hard --- metadata checksum is still pretty new and not
> fully supported, and the punch hole bug is again also pretty new
> functionality.
> 
> Also, pushing something to Linus now could potentially disrupt the
> patches in the ext4 dev tree for the next merge window.  For the zero
> extents problem, there's no question what are priorities should be;
> user dataloss trumps developer convenience any day.  For the rest,
> what do you all think?  Are they likely to hit people hard enough that
> it's worth trying to get this to Linus sooner, as opposed to having
> the fixes for the rest of the big endian issues land in 3.10 and
> 3.9.1?

Hi Ted,

I agree with you that we need to fix the big-endian bug in extent tree
because it affects all people who use ext4 file system.  Meanwhile I
think we need to fix the problem in punching hole and xattr because that
has been there.  I have looked at Dmitry's patch, and it fixes some
problems that is only in dev branch (e.g. migration).  I think this
problem can be fixed in 3.10 later.  Meanwhile metadata_csum is still
under ext4dev.  So I think it also can be fixed in next merge window.

Thanks,
                                                - Zheng

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ext4: fix cpu_vs_disk conversions
  2013-04-03 10:58 ` [PATCH 2/2] ext4: fix cpu_vs_disk conversions Dmitry Monakhov
  2013-04-03 12:33   ` Theodore Ts'o
@ 2013-04-03 15:37   ` Theodore Ts'o
  2013-04-03 15:55     ` Dmitry Monakhov
  2013-04-04 14:42   ` Dmitry Monakhov
  2013-04-10  4:01   ` Theodore Ts'o
  3 siblings, 1 reply; 16+ messages in thread
From: Theodore Ts'o @ 2013-04-03 15:37 UTC (permalink / raw)
  To: Dmitry Monakhov; +Cc: linux-ext4, wenqing.lz

On Wed, Apr 03, 2013 at 02:58:31PM +0400, Dmitry Monakhov wrote:
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 3a120b2..34da740 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -123,16 +123,15 @@ static __le32 ext4_xattr_block_csum(struct inode *inode,
>  {
>  	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
>  	__u32 csum, old;
> -
> -	old = hdr->h_checksum;
> +	__le64 dsk_block_nr = cpu_to_le64(block_nr);
> +	old = le32_to_cpu(hdr->h_checksum);

We're just saving and restoring hdr->h_checksum.  So instead of
byte-swapping the checksum in old, and then swapping it back, why not
just do this instead:

     __le32 old;

     old = hdr->h_checksum;
     ...
     hdr->h_checksum = old;

					- Ted

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ext4: fix cpu_vs_disk conversions
  2013-04-03 15:37   ` Theodore Ts'o
@ 2013-04-03 15:55     ` Dmitry Monakhov
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Monakhov @ 2013-04-03 15:55 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: linux-ext4, wenqing.lz

On Wed, 3 Apr 2013 11:37:42 -0400, Theodore Ts'o <tytso@mit.edu> wrote:
> On Wed, Apr 03, 2013 at 02:58:31PM +0400, Dmitry Monakhov wrote:
> > diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> > index 3a120b2..34da740 100644
> > --- a/fs/ext4/xattr.c
> > +++ b/fs/ext4/xattr.c
> > @@ -123,16 +123,15 @@ static __le32 ext4_xattr_block_csum(struct inode *inode,
> >  {
> >  	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> >  	__u32 csum, old;
> > -
> > -	old = hdr->h_checksum;
> > +	__le64 dsk_block_nr = cpu_to_le64(block_nr);
> > +	old = le32_to_cpu(hdr->h_checksum);
> 
> We're just saving and restoring hdr->h_checksum.  So instead of
> byte-swapping the checksum in old, and then swapping it back, why not
> just do this instead:
> 
>      __le32 old;
> 
>      old = hdr->h_checksum;
>      ...
>      hdr->h_checksum = old;
yes. obviously that is correct.
> 
> 					- Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ext4: fix cpu_vs_disk conversions
  2013-04-03 13:04     ` Zheng Liu
@ 2013-04-03 16:01       ` Theodore Ts'o
  0 siblings, 0 replies; 16+ messages in thread
From: Theodore Ts'o @ 2013-04-03 16:01 UTC (permalink / raw)
  To: Dmitry Monakhov, linux-ext4, wenqing.lz

On Wed, Apr 03, 2013 at 09:04:43PM +0800, Zheng Liu wrote:
> I agree with you that we need to fix the big-endian bug in extent tree
> because it affects all people who use ext4 file system.  Meanwhile I
> think we need to fix the problem in punching hole and xattr because that
> has been there.  I have looked at Dmitry's patch, and it fixes some
> problems that is only in dev branch (e.g. migration).  I think this
> problem can be fixed in 3.10 later.  Meanwhile metadata_csum is still
> under ext4dev.  So I think it also can be fixed in next merge window.

I took a closer look at the issues that Dmitry found in the checksum
code (including in xattr.c), and it doesn't actually fix any runtime
bugs.  It's mainly issues picked up by static code analyzers, but it's
not anything which is high priority to backport to stable kernels.

It's also the case that the zero_ex code was fine in 3.8; the bug
where we failed to introduce byte swapping was introduced in 3.9-rc1.

So the good news is that from what I can tell, there is no actual bug
leading to fs/data corruption relating to big endian systems in the
3.8.x kernels, and hence, the fixup patch will not need a
cc:stable@vger.kernel.org tag.

So what I am currently thinking about doing is fixing the actual bugs
(zero_ex and indirect punch hole) for 3.9-rc6.  The cleanup changes is
important so we can more easily find bugs using static code analyzers,
but that can be saved for the next merge window.  Sounds like a plan?

						- Ted

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ext4: fix cpu_vs_disk conversions
  2013-04-03 10:58 ` [PATCH 2/2] ext4: fix cpu_vs_disk conversions Dmitry Monakhov
  2013-04-03 12:33   ` Theodore Ts'o
  2013-04-03 15:37   ` Theodore Ts'o
@ 2013-04-04 14:42   ` Dmitry Monakhov
  2013-04-10  4:01   ` Theodore Ts'o
  3 siblings, 0 replies; 16+ messages in thread
From: Dmitry Monakhov @ 2013-04-04 14:42 UTC (permalink / raw)
  To: linux-ext4; +Cc: tytso, wenqing.lz

Today I've asked our admins about antiquarian hardware
Now I'm happy owner of:
1) Itanium2 : DualCore Processor 9020 x2 sockets, 12Gb Ram
   This one is not big-endian(at least linux does not support that) but
   it has 64k pages which allow hard core testing for 1kb fs block size :)
2) PowerMac: G5 powerpc x2 sockets, 2Gb (big endian arch)

I'll install debian on this irons this weekend. So let's hope
we next time will find issues earlier.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [1/2] ext4: fix usless declarations
  2013-04-03 10:58 [PATCH 1/2] ext4: fix usless declarations Dmitry Monakhov
  2013-04-03 10:58 ` [PATCH 2/2] ext4: fix cpu_vs_disk conversions Dmitry Monakhov
  2013-04-03 12:26 ` [PATCH 1/2] ext4: fix usless declarations Theodore Ts'o
@ 2013-04-10  2:50 ` Theodore Ts'o
  2 siblings, 0 replies; 16+ messages in thread
From: Theodore Ts'o @ 2013-04-10  2:50 UTC (permalink / raw)
  To: Dmitri Monakho; +Cc: linux-ext4, wenqing.lz

On Wed, Apr 03, 2013 at 12:58:30AM -0000, Dmitri Monakho wrote:
> This patch should fix sparse complains about shadow declatations.
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>

Applied to the dev branch, thanks.

						- Ted

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ext4: fix cpu_vs_disk conversions
  2013-04-03 10:58 ` [PATCH 2/2] ext4: fix cpu_vs_disk conversions Dmitry Monakhov
                     ` (2 preceding siblings ...)
  2013-04-04 14:42   ` Dmitry Monakhov
@ 2013-04-10  4:01   ` Theodore Ts'o
  2013-04-10  4:02     ` [PATCH 1/3] ext4: fix big-endian bug in extent migration code Theodore Ts'o
  3 siblings, 1 reply; 16+ messages in thread
From: Theodore Ts'o @ 2013-04-10  4:01 UTC (permalink / raw)
  To: Dmitry Monakhov; +Cc: linux-ext4, wenqing.lz

I've broken up this patch into three patches for ease of review.

     	       	    	       	     	     - Ted

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] ext4: fix big-endian bug in extent migration code
  2013-04-10  4:01   ` Theodore Ts'o
@ 2013-04-10  4:02     ` Theodore Ts'o
  2013-04-10  4:02       ` [PATCH 2/3] ext4: fix big-endian bug in metadata checksum calculations Theodore Ts'o
  2013-04-10  4:02       ` [PATCH 3/3] ext4: fix miscellaneous big endian warnings Theodore Ts'o
  0 siblings, 2 replies; 16+ messages in thread
From: Theodore Ts'o @ 2013-04-10  4:02 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Dmitry Monakhov, Theodore Ts'o, stable

From: Dmitry Monakhov <dmonakhov@openvz.org>

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: stable@vger.kernel.org
---
 fs/ext4/extents.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index c1c0094..fa85ce8 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4760,7 +4760,7 @@ int ext4_ind_migrate(struct inode *inode)
 	eh = ext_inode_hdr(inode);
 	ex  = EXT_FIRST_EXTENT(eh);
 	if (ext4_blocks_count(es) > EXT4_MAX_BLOCK_FILE_PHYS ||
-	    eh->eh_depth != 0 || eh->eh_entries > 1) {
+	    eh->eh_depth != 0 || le16_to_cpu(eh->eh_entries) > 1) {
 		ret = -EOPNOTSUPP;
 		goto errout;
 	}
-- 
1.7.12.rc0.22.gcdd159b

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] ext4: fix big-endian bug in metadata checksum calculations
  2013-04-10  4:02     ` [PATCH 1/3] ext4: fix big-endian bug in extent migration code Theodore Ts'o
@ 2013-04-10  4:02       ` Theodore Ts'o
  2013-04-10  4:02       ` [PATCH 3/3] ext4: fix miscellaneous big endian warnings Theodore Ts'o
  1 sibling, 0 replies; 16+ messages in thread
From: Theodore Ts'o @ 2013-04-10  4:02 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Dmitry Monakhov, Theodore Ts'o, stable

From: Dmitry Monakhov <dmonakhov@openvz.org>

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: stable@vger.kernel.org
---
 fs/ext4/inode.c | 8 ++++----
 fs/ext4/mmp.c   | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 629d67b..62189c8 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -55,21 +55,21 @@ static __u32 ext4_inode_csum(struct inode *inode, struct ext4_inode *raw,
 	__u16 csum_hi = 0;
 	__u32 csum;
 
-	csum_lo = raw->i_checksum_lo;
+	csum_lo = le16_to_cpu(raw->i_checksum_lo);
 	raw->i_checksum_lo = 0;
 	if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE &&
 	    EXT4_FITS_IN_INODE(raw, ei, i_checksum_hi)) {
-		csum_hi = raw->i_checksum_hi;
+		csum_hi = le16_to_cpu(raw->i_checksum_hi);
 		raw->i_checksum_hi = 0;
 	}
 
 	csum = ext4_chksum(sbi, ei->i_csum_seed, (__u8 *)raw,
 			   EXT4_INODE_SIZE(inode->i_sb));
 
-	raw->i_checksum_lo = csum_lo;
+	raw->i_checksum_lo = cpu_to_le16(csum_lo);
 	if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE &&
 	    EXT4_FITS_IN_INODE(raw, ei, i_checksum_hi))
-		raw->i_checksum_hi = csum_hi;
+		raw->i_checksum_hi = cpu_to_le16(csum_hi);
 
 	return csum;
 }
diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index f9b5515..b3b1f7d 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -7,7 +7,7 @@
 #include "ext4.h"
 
 /* Checksumming functions */
-static __u32 ext4_mmp_csum(struct super_block *sb, struct mmp_struct *mmp)
+static __le32 ext4_mmp_csum(struct super_block *sb, struct mmp_struct *mmp)
 {
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	int offset = offsetof(struct mmp_struct, mmp_checksum);
-- 
1.7.12.rc0.22.gcdd159b

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] ext4: fix miscellaneous big endian warnings
  2013-04-10  4:02     ` [PATCH 1/3] ext4: fix big-endian bug in extent migration code Theodore Ts'o
  2013-04-10  4:02       ` [PATCH 2/3] ext4: fix big-endian bug in metadata checksum calculations Theodore Ts'o
@ 2013-04-10  4:02       ` Theodore Ts'o
  1 sibling, 0 replies; 16+ messages in thread
From: Theodore Ts'o @ 2013-04-10  4:02 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

None of these result in any bug, but they makes sparse complain.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/namei.c             |  7 ++++---
 fs/ext4/super.c             |  6 +++---
 fs/ext4/xattr.c             | 13 +++++++------
 include/trace/events/ext4.h |  6 +++---
 4 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index ede1337..955c907 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -416,15 +416,16 @@ static __le32 ext4_dx_csum(struct inode *inode, struct ext4_dir_entry *dirent,
 {
 	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
 	struct ext4_inode_info *ei = EXT4_I(inode);
-	__u32 csum, old_csum;
+	__u32 csum;
+	__le32 save_csum;
 	int size;
 
 	size = count_offset + (count * sizeof(struct dx_entry));
-	old_csum = t->dt_checksum;
+	save_csum = t->dt_checksum;
 	t->dt_checksum = 0;
 	csum = ext4_chksum(sbi, ei->i_csum_seed, (__u8 *)dirent, size);
 	csum = ext4_chksum(sbi, csum, (__u8 *)t, sizeof(struct dx_tail));
-	t->dt_checksum = old_csum;
+	t->dt_checksum = save_csum;
 
 	return cpu_to_le32(csum);
 }
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 6fea87d..f355c28 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1952,16 +1952,16 @@ static __le16 ext4_group_desc_csum(struct ext4_sb_info *sbi, __u32 block_group,
 	if ((sbi->s_es->s_feature_ro_compat &
 	     cpu_to_le32(EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))) {
 		/* Use new metadata_csum algorithm */
-		__u16 old_csum;
+		__le16 save_csum;
 		__u32 csum32;
 
-		old_csum = gdp->bg_checksum;
+		save_csum = gdp->bg_checksum;
 		gdp->bg_checksum = 0;
 		csum32 = ext4_chksum(sbi, sbi->s_csum_seed, (__u8 *)&le_group,
 				     sizeof(le_group));
 		csum32 = ext4_chksum(sbi, csum32, (__u8 *)gdp,
 				     sbi->s_desc_size);
-		gdp->bg_checksum = old_csum;
+		gdp->bg_checksum = save_csum;
 
 		crc = csum32 & 0xFFFF;
 		goto out;
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 3a120b2..c081e34 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -122,17 +122,18 @@ static __le32 ext4_xattr_block_csum(struct inode *inode,
 				    struct ext4_xattr_header *hdr)
 {
 	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
-	__u32 csum, old;
+	__u32 csum;
+	__le32 save_csum;
+	__le64 dsk_block_nr = cpu_to_le64(block_nr);
 
-	old = hdr->h_checksum;
+	save_csum = hdr->h_checksum;
 	hdr->h_checksum = 0;
-	block_nr = cpu_to_le64(block_nr);
-	csum = ext4_chksum(sbi, sbi->s_csum_seed, (__u8 *)&block_nr,
-			   sizeof(block_nr));
+	csum = ext4_chksum(sbi, sbi->s_csum_seed, (__u8 *)&dsk_block_nr,
+			   sizeof(dsk_block_nr));
 	csum = ext4_chksum(sbi, csum, (__u8 *)hdr,
 			   EXT4_BLOCK_SIZE(inode->i_sb));
 
-	hdr->h_checksum = old;
+	hdr->h_checksum = save_csum;
 	return cpu_to_le32(csum);
 }
 
diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
index 58459b7..d0e6864 100644
--- a/include/trace/events/ext4.h
+++ b/include/trace/events/ext4.h
@@ -1948,7 +1948,7 @@ TRACE_EVENT(ext4_remove_blocks,
 		__entry->to		= to;
 		__entry->partial	= partial_cluster;
 		__entry->ee_pblk	= ext4_ext_pblock(ex);
-		__entry->ee_lblk	= cpu_to_le32(ex->ee_block);
+		__entry->ee_lblk	= le32_to_cpu(ex->ee_block);
 		__entry->ee_len		= ext4_ext_get_actual_len(ex);
 	),
 
@@ -2052,7 +2052,7 @@ TRACE_EVENT(ext4_ext_remove_space,
 
 TRACE_EVENT(ext4_ext_remove_space_done,
 	TP_PROTO(struct inode *inode, ext4_lblk_t start, int depth,
-		ext4_lblk_t partial, unsigned short eh_entries),
+		ext4_lblk_t partial, __le16 eh_entries),
 
 	TP_ARGS(inode, start, depth, partial, eh_entries),
 
@@ -2071,7 +2071,7 @@ TRACE_EVENT(ext4_ext_remove_space_done,
 		__entry->start		= start;
 		__entry->depth		= depth;
 		__entry->partial	= partial;
-		__entry->eh_entries	= eh_entries;
+		__entry->eh_entries	= le16_to_cpu(eh_entries);
 	),
 
 	TP_printk("dev %d,%d ino %lu since %u depth %d partial %u "
-- 
1.7.12.rc0.22.gcdd159b


^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2013-04-10  4:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-03 10:58 [PATCH 1/2] ext4: fix usless declarations Dmitry Monakhov
2013-04-03 10:58 ` [PATCH 2/2] ext4: fix cpu_vs_disk conversions Dmitry Monakhov
2013-04-03 12:33   ` Theodore Ts'o
2013-04-03 12:58     ` Dmitry Monakhov
2013-04-03 13:04     ` Zheng Liu
2013-04-03 16:01       ` Theodore Ts'o
2013-04-03 15:37   ` Theodore Ts'o
2013-04-03 15:55     ` Dmitry Monakhov
2013-04-04 14:42   ` Dmitry Monakhov
2013-04-10  4:01   ` Theodore Ts'o
2013-04-10  4:02     ` [PATCH 1/3] ext4: fix big-endian bug in extent migration code Theodore Ts'o
2013-04-10  4:02       ` [PATCH 2/3] ext4: fix big-endian bug in metadata checksum calculations Theodore Ts'o
2013-04-10  4:02       ` [PATCH 3/3] ext4: fix miscellaneous big endian warnings Theodore Ts'o
2013-04-03 12:26 ` [PATCH 1/2] ext4: fix usless declarations Theodore Ts'o
2013-04-03 12:31   ` Dmitry Monakhov
2013-04-10  2:50 ` [1/2] " Theodore Ts'o

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.