Skip to contentSkip to conversation

Fix csum error for compressed data

PATCH In review Yan Zheng 3 participants 8 messages Nov 10, 2008 – Nov 11, 2008 Btrfs file system

lore
Share & tools
lore

Yan Zheng

author
#0 lore raw

PATCH 1/1 Fix csum error for compressed data

Hello,

The decompress code doesn't take the logical offset in extent pointer into account. If the logical offset isn't zero, data will be decompressed into wrong pages. Thank you,

---

fs/btrfs/compression.c +2 -4
diff -urp 1/fs/btrfs/compression.c 2/fs/btrfs/compression.c
@@ -505,7 +505,6 @@ int btrfs_submit_compressed_read(struct 
 	struct block_device *bdev;
 	struct bio *comp_bio;
 	u64 cur_disk_byte = (u64)bio->bi_sector << 9;
-	u64 em_len;
 	struct extent_map *em;
 	int ret;
 
@@ -524,9 +523,8 @@ int btrfs_submit_compressed_read(struct 
 	cb->errors = 0;
 	cb->inode = inode;
 
-	cb->start = em->start;
+	cb->start = em->orig_start;
 	compressed_len = em->block_len;
-	em_len = em->len;
 	free_extent_map(em);
 
 	cb->len = uncompressed_len;
@@ -545,7 +543,7 @@ int btrfs_submit_compressed_read(struct 
 	}
 	cb->nr_pages = nr_pages;
 
-	add_ra_bio_pages(inode, cb->start + em_len, cb);
+	add_ra_bio_pages(inode, em->start + em->len, cb);
 
 	if (!btrfs_test_opt(root, NODATASUM) &&
 	    !btrfs_test_flag(inode, NODATASUM)) {
fs/btrfs/extent_map.h +1 -0
diff -urp 1/fs/btrfs/extent_map.h 2/fs/btrfs/extent_map.h
@@ -20,6 +20,7 @@ struct extent_map {
 	/* all of these are in bytes */
 	u64 start;
 	u64 len;
+	u64 orig_start;
 	u64 block_start;
 	u64 block_len;
 	unsigned long flags;
fs/btrfs/file.c +2 -0
diff -urp 1/fs/btrfs/file.c 2/fs/btrfs/file.c
@@ -222,6 +222,7 @@ int btrfs_drop_extent_cache(struct inode
 		    em->start < start) {
 			split->start = em->start;
 			split->len = start - em->start;
+			split->orig_start = em->orig_start;
 			split->block_start = em->block_start;
 
 			if (compressed)
@@ -243,6 +244,7 @@ int btrfs_drop_extent_cache(struct inode
 
 			split->start = start + len;
 			split->len = em->start + em->len - (start + len);
+			split->orig_start = em->orig_start;
 			split->bdev = em->bdev;
 			split->flags = flags;
 
fs/btrfs/inode.c +3 -0
diff -urp 1/fs/btrfs/inode.c 2/fs/btrfs/inode.c
@@ -3949,6 +3949,8 @@ again:
 	    found_type == BTRFS_FILE_EXTENT_PREALLOC) {
 		em->start = extent_start;
 		em->len = extent_end - extent_start;
+		em->orig_start = extent_start -
+				 btrfs_file_extent_offset(leaf, item);
 		bytenr = btrfs_file_extent_disk_bytenr(leaf, item);
 		if (bytenr == 0) {
 			em->block_start = EXTENT_MAP_HOLE;
@@ -3988,6 +3990,7 @@ again:
 		em->start = extent_start + extent_offset;
 		em->len = (copy_size + root->sectorsize - 1) &
 			~((u64)root->sectorsize - 1);
+		em->orig_start = EXTENT_MAP_INLINE;
 		if (compressed)
 			set_bit(EXTENT_FLAG_COMPRESSED, &em->flags);
 		ptr = btrfs_file_extent_inline_start(item) + extent_offset;
#1 lore raw
Quoted 6 lines · Yan Zheng

Hello,

The decompress code doesn't take the logical offset in extent pointer into account. If the logical offset isn't zero, data will be decompressed into wrong pages. Thank you,

Ah, thanks Yan.

I'll push this out along with a few other fixes from the weekend shortly.

-chris

#2 lore raw
Quoted 7 lines · Chris Mason
Quoted 5 lines · Yan Zheng

Hello,

The decompress code doesn't take the logical offset in extent pointer into account. If the logical offset isn't zero, data will be decompressed into wrong pages. Thank you,

Quoted 5 lines

Ah, thanks Yan.

I'll push this out along with a few other fixes from the weekend shortly.

Well, if this should fix the issue I've reported last Friday I have=20 to tell you that this error still occurs during my test drives.

Best wishes,
--=20
Martin B=C3=BCrger 1024D/[email protected]/~mbuerger/key.asc
Key fingerprint =3D D10D 97EF 0C32 B337 5A12 0B6C 2D47 7575 27C9 019B --
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" = in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
Quoted 8 lines · On Mon, 2008-11-10 at 22:00 +0100, Martin B=C3=BCrger wrote:
Quoted 7 lines · Chris Mason
Quoted 5 lines · Yan Zheng

Hello,

The decompress code doesn't take the logical offset in extent pointer into account. If the logical offset isn't zero, data will be decompressed into wrong pages. Thank you,

Quoted 7 lines
Quoted 5 lines

Ah, thanks Yan.

I'll push this out along with a few other fixes from the weekend shortly.

Well, if this should fix the issue I've reported last Friday I have=20 to tell you that this error still occurs during my test drives.

Do you also have this changeset:

http://git.kernel.org/?p=3Dlinux/kernel/git/mason/btrfs-unstable.git;a=3D=
commit;h=3De04ca626baee684bea9d6239e4e1119b696101b2

-chris

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" = in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#4 lore raw
Quoted 16 lines · Chris Mason
Quoted 15 lines · On Mon, 2008-11-10 at 22:00 +0100, Martin B=C3=BCrger wrote:
Quoted 13 lines · Chris Mason
Quoted 6 lines · Yan Zheng

Hello,

The decompress code doesn't take the logical offset in extent pointer into account. If the logical offset isn't zero, data will be decompressed into wrong pages. Thank you,

Ah, thanks Yan.

I'll push this out along with a few other fixes from the weekend shortly.

Quoted 8 lines
Quoted 3 lines

Well, if this should fix the issue I've reported last Friday I have to tell you that this error still occurs during my test drives.

Do you also have this changeset:

http://git.kernel.org/?p=3Dlinux/kernel/git/mason/btrfs-unstable.gi
t;a=3Dcommit;h=3De04ca626baee684bea9d6239e4e1119b696101b2

Yes, I do.

--=20
Martin B=C3=BCrger 1024D/[email protected]/~mbuerger/key.asc
Key fingerprint =3D D10D 97EF 0C32 B337 5A12 0B6C 2D47 7575 27C9 019B --
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" = in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#5 lore raw

Hi Martin,

Please try following patch. Thank you,

---

fs/btrfs/file.c +1 -1
diff -urp 2/fs/btrfs/file.c 5/fs/btrfs/file.c
@@ -436,7 +436,7 @@ next_slot:
 			goto out;
 		}
 		if (recow) {
-			search_start = key.offset;
+			search_start = max(key.offset, start);
 			continue;
 		}
 		if (btrfs_key_type(&key) == BTRFS_EXTENT_DATA_KEY) {
#7 lore raw
Quoted 5 lines · On Tue, 2008-11-11 at 18:14 +0100, Martin B=C3=BCrger wrote:
Quoted 3 lines · Yan Zheng

Hi Martin,

Please try following patch. Thank you,

That fixed it.

Thank you both, I've pushed out Yan Zheng's fix.

-chris

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" = in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html