Problems with hfsplus on ipods in 2.6.38+

Problems with hfsplus on ipods in 2.6.38+

[Daniel Barkalow]

I've got an ipod which doesn't work with 2.6.38 or later, unless I revert:

358f26d52680cb150907302d4334359de7dd2d59
52399b171dfaea02b6944cd6feba49b624147126

In the failing case, I get:

 sd 2:0:0:0: [sdb] Bad block number requested
 hfs: unable to find HFS+ superblock

in dmesg when I try to mount it.

Before 2.6.38, or with those commits reverted, it mounts fine and works so 
far as I can tell. (There's an Ubuntu bug report: 
https://bugs.launchpad.net/ubuntu/+source/util-linux/+bug/734883, which 
reports other people having similar results). I can test patches and 
collect information that might be helpful, if you have any ideas.

	-Daniel
*This .sig left intentionally blank*

Re: Problems with hfsplus on ipods in 2.6.38+

[Seth Forshee]

On Wed, Jul 13, 2011 at 10:10:13PM -0400, Daniel Barkalow wrote:
> I've got an ipod which doesn't work with 2.6.38 or later, unless I revert:
> 
> 358f26d52680cb150907302d4334359de7dd2d59
> 52399b171dfaea02b6944cd6feba49b624147126
> 
> In the failing case, I get:
> 
>  sd 2:0:0:0: [sdb] Bad block number requested
>  hfs: unable to find HFS+ superblock
> 
> in dmesg when I try to mount it.
> 
> Before 2.6.38, or with those commits reverted, it mounts fine and works so 
> far as I can tell. (There's an Ubuntu bug report: 
> https://bugs.launchpad.net/ubuntu/+source/util-linux/+bug/734883, which 
> reports other people having similar results). I can test patches and 
> collect information that might be helpful, if you have any ideas.

This has had some discussion on lkml previously [1]. I've been trying to
work on a fix, but the current iteration has problems, and I haven't
been able to get testers to provide the logs I've requested to help me
see what's going wrong. If you'd like to help by providing logs from the
most recent test build on the bug you linked to (or using the patches
from that build) that would be great. I unfortunately don't have any
large-sector devices to test with and cannot reproduce the problems on
512-byte sector devices.

Seth

[1] https://lkml.org/lkml/2011/5/25/248

Re: Problems with hfsplus on ipods in 2.6.38+

[Christoph Hellwig]

On Thu, Jul 14, 2011 at 08:53:02AM -0500, Seth Forshee wrote:
> This has had some discussion on lkml previously [1]. I've been trying to
> work on a fix, but the current iteration has problems, and I haven't
> been able to get testers to provide the logs I've requested to help me
> see what's going wrong. If you'd like to help by providing logs from the
> most recent test build on the bug you linked to (or using the patches
> from that build) that would be great. I unfortunately don't have any
> large-sector devices to test with and cannot reproduce the problems on
> 512-byte sector devices.

I've been trying to get a large block size test device using qemu
or scsi_debug but haven't managed to get back to it.  If you can get
Daniel to test you're patches I'd really appreciate your help.

Re: Problems with hfsplus on ipods in 2.6.38+

[Seth Forshee]

On Thu, Jul 14, 2011 at 10:45:14AM -0400, Christoph Hellwig wrote:
> On Thu, Jul 14, 2011 at 08:53:02AM -0500, Seth Forshee wrote:
> > This has had some discussion on lkml previously [1]. I've been trying to
> > work on a fix, but the current iteration has problems, and I haven't
> > been able to get testers to provide the logs I've requested to help me
> > see what's going wrong. If you'd like to help by providing logs from the
> > most recent test build on the bug you linked to (or using the patches
> > from that build) that would be great. I unfortunately don't have any
> > large-sector devices to test with and cannot reproduce the problems on
> > 512-byte sector devices.
> 
> I've been trying to get a large block size test device using qemu
> or scsi_debug but haven't managed to get back to it.  If you can get
> Daniel to test you're patches I'd really appreciate your help.

I wasn't aware of scsi_debug. I'll also try to test using that and see
if I can recreate the problems.

Re: Problems with hfsplus on ipods in 2.6.38+

[Daniel Barkalow]

On Thu, 14 Jul 2011, Seth Forshee wrote:

> On Wed, Jul 13, 2011 at 10:10:13PM -0400, Daniel Barkalow wrote:
> > I've got an ipod which doesn't work with 2.6.38 or later, unless I revert:
> > 
> > 358f26d52680cb150907302d4334359de7dd2d59
> > 52399b171dfaea02b6944cd6feba49b624147126
> > 
> > In the failing case, I get:
> > 
> >  sd 2:0:0:0: [sdb] Bad block number requested
> >  hfs: unable to find HFS+ superblock
> > 
> > in dmesg when I try to mount it.
> > 
> > Before 2.6.38, or with those commits reverted, it mounts fine and works so 
> > far as I can tell. (There's an Ubuntu bug report: 
> > https://bugs.launchpad.net/ubuntu/+source/util-linux/+bug/734883, which 
> > reports other people having similar results). I can test patches and 
> > collect information that might be helpful, if you have any ideas.
> 
> This has had some discussion on lkml previously [1]. I've been trying to
> work on a fix, but the current iteration has problems, and I haven't
> been able to get testers to provide the logs I've requested to help me
> see what's going wrong. If you'd like to help by providing logs from the
> most recent test build on the bug you linked to (or using the patches
> from that build) that would be great. I unfortunately don't have any
> large-sector devices to test with and cannot reproduce the problems on
> 512-byte sector devices.

Sure, I'll try the comment #36 patch set this evening and see what it says.

	-Daniel
*This .sig left intentionally blank*

Re: Problems with hfsplus on ipods in 2.6.38+

[Daniel Barkalow]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1898 bytes --]

On Thu, 14 Jul 2011, Daniel Barkalow wrote:

> On Thu, 14 Jul 2011, Seth Forshee wrote:
> 
> > On Wed, Jul 13, 2011 at 10:10:13PM -0400, Daniel Barkalow wrote:
> > > I've got an ipod which doesn't work with 2.6.38 or later, unless I revert:
> > > 
> > > 358f26d52680cb150907302d4334359de7dd2d59
> > > 52399b171dfaea02b6944cd6feba49b624147126
> > > 
> > > In the failing case, I get:
> > > 
> > >  sd 2:0:0:0: [sdb] Bad block number requested
> > >  hfs: unable to find HFS+ superblock
> > > 
> > > in dmesg when I try to mount it.
> > > 
> > > Before 2.6.38, or with those commits reverted, it mounts fine and works so 
> > > far as I can tell. (There's an Ubuntu bug report: 
> > > https://bugs.launchpad.net/ubuntu/+source/util-linux/+bug/734883, which 
> > > reports other people having similar results). I can test patches and 
> > > collect information that might be helpful, if you have any ideas.
> > 
> > This has had some discussion on lkml previously [1]. I've been trying to
> > work on a fix, but the current iteration has problems, and I haven't
> > been able to get testers to provide the logs I've requested to help me
> > see what's going wrong. If you'd like to help by providing logs from the
> > most recent test build on the bug you linked to (or using the patches
> > from that build) that would be great. I unfortunately don't have any
> > large-sector devices to test with and cannot reproduce the problems on
> > 512-byte sector devices.
> 
> Sure, I'll try the comment #36 patch set this evening and see what it says.

Okay, I've applied that patch set, and it worked for me without any issues 
thus far. If you're interested in the debugging output from a device that 
doesn't work with vanilla but doesn't oops or panic with that patch set, 
it's attached. I'm using 32-bit x86, if that helps for tracking down 
differences.

	-Daniel
*This .sig left intentionally blank*

[-- Attachment #2: Type: TEXT/PLAIN, Size: 2976 bytes --]

usb 1-3: new high speed USB device using ehci_hcd and address 3
scsi2 : usb-storage 1-3:1.0
scsi 2:0:0:0: Direct-Access     Apple    iPod             1.62 PQ: 0 ANSI: 0
sd 2:0:0:0: Attached scsi generic sg2 type 0
sd 2:0:0:0: [sdb] 3964928 2048-byte logical blocks: (8.12 GB/7.56 GiB)
sd 2:0:0:0: [sdb] Write Protect is off
sd 2:0:0:0: [sdb] Mode Sense: 68 00 00 08
sd 2:0:0:0: [sdb] Assuming drive cache: write through
sd 2:0:0:0: [sdb] 3964928 2048-byte logical blocks: (8.12 GB/7.56 GiB)
sd 2:0:0:0: [sdb] Assuming drive cache: write through
 sdb: [mac] sdb1 sdb2 sdb3
sd 2:0:0:0: [sdb] 3964928 2048-byte logical blocks: (8.12 GB/7.56 GiB)
sd 2:0:0:0: [sdb] Assuming drive cache: write through
sd 2:0:0:0: [sdb] Attached SCSI removable disk
SAF: hfsplus_submit_bio: sector 2 buf   (null) rw -183074816
SAF: hfsplus_submit_bio: io_size 2048 start 1024 offset 1024 sector 0
SAF: hfsplus_submit_bio: data f506d544
SAF: hfsplus_submit_bio: io_size 2048 page_offset 0 len 2048 buf f5168000
SAF: hfsplus_submit_bio: sector 15615178 buf   (null) rw -180664320
SAF: hfsplus_submit_bio: io_size 2048 start 7994971136 offset 1024 sector 15615176
SAF: hfsplus_submit_bio: data f506d54c
SAF: hfsplus_submit_bio: io_size 2048 page_offset 2048 len 2048 buf f53b4800
hfs: write access to a journaled filesystem is not supported, use the force option at your own risk, mounting read-only.
SAF: hfsplus_submit_bio: sector 2 buf   (null) rw -196237312
SAF: hfsplus_submit_bio: io_size 2048 start 1024 offset 1024 sector 0
SAF: hfsplus_submit_bio: data f506d544
SAF: hfsplus_submit_bio: io_size 2048 page_offset 2048 len 2048 buf f44da800
SAF: hfsplus_submit_bio: sector 15615178 buf   (null) rw -173721600
SAF: hfsplus_submit_bio: io_size 2048 start 7994971136 offset 1024 sector 15615176
SAF: hfsplus_submit_bio: data f506d54c
SAF: hfsplus_submit_bio: io_size 2048 page_offset 2048 len 2048 buf f5a53800
SAF: hfsplus_submit_bio: sector 2 buf   (null) rw -196237312
SAF: hfsplus_submit_bio: io_size 2048 start 1024 offset 1024 sector 0
SAF: hfsplus_submit_bio: io_size 2048 page_offset 2048 len 2048 buf f44da800
SAF: hfsplus_submit_bio: sector 2 buf   (null) rw -196237312
SAF: hfsplus_submit_bio: io_size 2048 start 1024 offset 1024 sector 0
SAF: hfsplus_submit_bio: io_size 2048 page_offset 2048 len 2048 buf f44da800
SAF: hfsplus_submit_bio: sector 2 buf   (null) rw -196237312
SAF: hfsplus_submit_bio: io_size 2048 start 1024 offset 1024 sector 0
SAF: hfsplus_submit_bio: io_size 2048 page_offset 2048 len 2048 buf f44da800
SAF: hfsplus_submit_bio: sector 2 buf   (null) rw -196237312
SAF: hfsplus_submit_bio: io_size 2048 start 1024 offset 1024 sector 0
SAF: hfsplus_submit_bio: io_size 2048 page_offset 2048 len 2048 buf f44da800
SAF: hfsplus_submit_bio: sector 2 buf   (null) rw -196237312
SAF: hfsplus_submit_bio: io_size 2048 start 1024 offset 1024 sector 0
SAF: hfsplus_submit_bio: io_size 2048 page_offset 2048 len 2048 buf f44da800

Re: Problems with hfsplus on ipods in 2.6.38+

[Seth Forshee]

On Thu, Jul 14, 2011 at 09:26:11PM -0400, Daniel Barkalow wrote:
> Okay, I've applied that patch set, and it worked for me without any issues 
> thus far. If you're interested in the debugging output from a device that 
> doesn't work with vanilla but doesn't oops or panic with that patch set, 
> it's attached. I'm using 32-bit x86, if that helps for tracking down 
> differences.

Hrm, looks like I used %lu for sector_t instead of %llu, and that's
messing up the output on 32-bit builds. What I am able to see looks
correct though. I put up a new version of the patches with the output
fixed along with a new build on the bug.

I've had some success producing problems using scsi_debug with a 64-bit
build, specifically with 1K or 2K sectors. Actually a lot of odd things
happen with those sector sizes, and they happen whether using my patch
or reverting the two patches that change hfsplus to using bio, so those
problems seem unrelated. What I see is that the free/used space numbers
reported by df don't make sense given the actual files I've copied to
the volume. If I "fill" the volume (in quotes because really I haven't
copied in enough data to fill the volume, but it says it's full anyway)
df reports complete garbage. Then if I proceed to remove all files from
the volume df still reports that 50% of the space is used. These
problems aren't present with 512 byte or 4K sectors.

What I also see are GPFs in memory allocation code, which is what I
believe others have seen with my patch, and so far I haven't seen those
with the reversions. So I'm suspecting memory corruption, but I don't
yet see where the corruption is coming form. I found one problem, but I
don't suspect it's responsible for the GPFs.

I'm still investigating, but I won't really be able to spend much more
time on it until Monday.

Seth

Re: Problems with hfsplus on ipods in 2.6.38+

[Daniel Barkalow]

On Fri, 15 Jul 2011, Seth Forshee wrote:

> On Thu, Jul 14, 2011 at 09:26:11PM -0400, Daniel Barkalow wrote:
> > Okay, I've applied that patch set, and it worked for me without any issues 
> > thus far. If you're interested in the debugging output from a device that 
> > doesn't work with vanilla but doesn't oops or panic with that patch set, 
> > it's attached. I'm using 32-bit x86, if that helps for tracking down 
> > differences.
> 
> Hrm, looks like I used %lu for sector_t instead of %llu, and that's
> messing up the output on 32-bit builds. What I am able to see looks
> correct though. I put up a new version of the patches with the output
> fixed along with a new build on the bug.
> 
> I've had some success producing problems using scsi_debug with a 64-bit
> build, specifically with 1K or 2K sectors. Actually a lot of odd things
> happen with those sector sizes, and they happen whether using my patch
> or reverting the two patches that change hfsplus to using bio, so those
> problems seem unrelated. What I see is that the free/used space numbers
> reported by df don't make sense given the actual files I've copied to
> the volume. If I "fill" the volume (in quotes because really I haven't
> copied in enough data to fill the volume, but it says it's full anyway)
> df reports complete garbage. Then if I proceed to remove all files from
> the volume df still reports that 50% of the space is used. These
> problems aren't present with 512 byte or 4K sectors.
> 
> What I also see are GPFs in memory allocation code, which is what I
> believe others have seen with my patch, and so far I haven't seen those
> with the reversions. So I'm suspecting memory corruption, but I don't
> yet see where the corruption is coming form. I found one problem, but I
> don't suspect it's responsible for the GPFs.

A while later, somewhat after I'd unmounted the filesystem (and sent the 
email), I got some memory allocation oopses, also, followed eventually by 
some sort of hang (userspace not working but alt-sysrq did work). So I 
agree with the memory corruption idea. Do you want corrected debugging 
output, or any other information from my actual device, or are you set 
with scsi_debug for now?

	-Daniel
*This .sig left intentionally blank*

Re: Problems with hfsplus on ipods in 2.6.38+

[Seth Forshee]

On Fri, Jul 15, 2011 at 11:43:47AM -0400, Daniel Barkalow wrote:
> On Fri, 15 Jul 2011, Seth Forshee wrote:
> 
> > On Thu, Jul 14, 2011 at 09:26:11PM -0400, Daniel Barkalow wrote:
> > > Okay, I've applied that patch set, and it worked for me without any issues 
> > > thus far. If you're interested in the debugging output from a device that 
> > > doesn't work with vanilla but doesn't oops or panic with that patch set, 
> > > it's attached. I'm using 32-bit x86, if that helps for tracking down 
> > > differences.
> > 
> > Hrm, looks like I used %lu for sector_t instead of %llu, and that's
> > messing up the output on 32-bit builds. What I am able to see looks
> > correct though. I put up a new version of the patches with the output
> > fixed along with a new build on the bug.
> > 
> > I've had some success producing problems using scsi_debug with a 64-bit
> > build, specifically with 1K or 2K sectors. Actually a lot of odd things
> > happen with those sector sizes, and they happen whether using my patch
> > or reverting the two patches that change hfsplus to using bio, so those
> > problems seem unrelated. What I see is that the free/used space numbers
> > reported by df don't make sense given the actual files I've copied to
> > the volume. If I "fill" the volume (in quotes because really I haven't
> > copied in enough data to fill the volume, but it says it's full anyway)
> > df reports complete garbage. Then if I proceed to remove all files from
> > the volume df still reports that 50% of the space is used. These
> > problems aren't present with 512 byte or 4K sectors.
> > 
> > What I also see are GPFs in memory allocation code, which is what I
> > believe others have seen with my patch, and so far I haven't seen those
> > with the reversions. So I'm suspecting memory corruption, but I don't
> > yet see where the corruption is coming form. I found one problem, but I
> > don't suspect it's responsible for the GPFs.
> 
> A while later, somewhat after I'd unmounted the filesystem (and sent the 
> email), I got some memory allocation oopses, also, followed eventually by 
> some sort of hang (userspace not working but alt-sysrq did work). So I 
> agree with the memory corruption idea. Do you want corrected debugging 
> output, or any other information from my actual device, or are you set 
> with scsi_debug for now?

I think I'm okay with scsi_debug. What would be most helpful now is a
deterministic way to reproduce the oopses.

Re: Problems with hfsplus on ipods in 2.6.38+

[Seth Forshee]

On Fri, Jul 15, 2011 at 09:39:00AM -0500, Seth Forshee wrote:
> What I also see are GPFs in memory allocation code, which is what I
> believe others have seen with my patch, and so far I haven't seen those
> with the reversions. So I'm suspecting memory corruption, but I don't
> yet see where the corruption is coming form. I found one problem, but I
> don't suspect it's responsible for the GPFs.

I had some torture tests running over the weekend with the "one
problem" mentioned above fixed, which was a couple of kfrees that were
still using the old vhdr pointers. There were no oopses over a couple of
days of testing. I've posted a new build on the bug in launchpad in
hopes of getting some testing from those who were getting oopses quite
readily. The updated patch follows.


>From fee1338c82f15b1558f960e5024948bd460af6c8 Mon Sep 17 00:00:00 2001
From: Seth Forshee <seth.forshee@canonical.com>
Date: Fri, 27 May 2011 15:10:51 -0500
Subject: [PATCH] hfsplus: ensure bio requests are not smaller than the hardware sectors

Currently all bio requests are 512 bytes, which may fail for media
whose physical sector size is larger than this. Ensure these
requests are not smaller than the block device logical block size.

BugLink: http://bugs.launchpad.net/bugs/734883
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 fs/hfsplus/hfsplus_fs.h |   16 ++++++++-
 fs/hfsplus/part_tbl.c   |   32 ++++++++++--------
 fs/hfsplus/super.c      |   12 +++---
 fs/hfsplus/wrapper.c    |   83 +++++++++++++++++++++++++++++++++++-----------
 4 files changed, 101 insertions(+), 42 deletions(-)

diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index d685752..4e7f64b 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -13,6 +13,7 @@
 #include <linux/fs.h>
 #include <linux/mutex.h>
 #include <linux/buffer_head.h>
+#include <linux/blkdev.h>
 #include "hfsplus_raw.h"
 
 #define DBG_BNODE_REFS	0x00000001
@@ -110,7 +111,9 @@ struct hfsplus_vh;
 struct hfs_btree;
 
 struct hfsplus_sb_info {
+	void *s_vhdr_buf;
 	struct hfsplus_vh *s_vhdr;
+	void *s_backup_vhdr_buf;
 	struct hfsplus_vh *s_backup_vhdr;
 	struct hfs_btree *ext_tree;
 	struct hfs_btree *cat_tree;
@@ -258,6 +261,15 @@ struct hfsplus_readdir_data {
 	struct hfsplus_cat_key key;
 };
 
+/*
+ * Find minimum acceptible I/O size for an hfsplus sb.
+ */
+static inline unsigned short hfsplus_min_io_size(struct super_block *sb)
+{
+	return max_t(unsigned short, bdev_logical_block_size(sb->s_bdev),
+		     HFSPLUS_SECTOR_SIZE);
+}
+
 #define hfs_btree_open hfsplus_btree_open
 #define hfs_btree_close hfsplus_btree_close
 #define hfs_btree_write hfsplus_btree_write
@@ -436,8 +448,8 @@ int hfsplus_compare_dentry(const struct dentry *parent,
 /* wrapper.c */
 int hfsplus_read_wrapper(struct super_block *);
 int hfs_part_find(struct super_block *, sector_t *, sector_t *);
-int hfsplus_submit_bio(struct block_device *bdev, sector_t sector,
-		void *data, int rw);
+int hfsplus_submit_bio(struct super_block *sb, sector_t sector,
+		void *buf, void **data, int rw);
 
 /* time macros */
 #define __hfsp_mt2ut(t)		(be32_to_cpu(t) - 2082844800U)
diff --git a/fs/hfsplus/part_tbl.c b/fs/hfsplus/part_tbl.c
index 40ad88c..eb355d8 100644
--- a/fs/hfsplus/part_tbl.c
+++ b/fs/hfsplus/part_tbl.c
@@ -88,11 +88,12 @@ static int hfs_parse_old_pmap(struct super_block *sb, struct old_pmap *pm,
 	return -ENOENT;
 }
 
-static int hfs_parse_new_pmap(struct super_block *sb, struct new_pmap *pm,
-		sector_t *part_start, sector_t *part_size)
+static int hfs_parse_new_pmap(struct super_block *sb, void *buf,
+		struct new_pmap *pm, sector_t *part_start, sector_t *part_size)
 {
 	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
 	int size = be32_to_cpu(pm->pmMapBlkCnt);
+	int buf_size = hfsplus_min_io_size(sb);
 	int res;
 	int i = 0;
 
@@ -107,11 +108,14 @@ static int hfs_parse_new_pmap(struct super_block *sb, struct new_pmap *pm,
 		if (++i >= size)
 			return -ENOENT;
 
-		res = hfsplus_submit_bio(sb->s_bdev,
-					 *part_start + HFS_PMAP_BLK + i,
-					 pm, READ);
-		if (res)
-			return res;
+		pm = (struct new_pmap *)((u8 *)pm + HFSPLUS_SECTOR_SIZE);
+		if ((u8 *)pm - (u8 *)buf >= buf_size) {
+			res = hfsplus_submit_bio(sb,
+						 *part_start + HFS_PMAP_BLK + i,
+						 buf, (void **)&pm, READ);
+			if (res)
+				return res;
+		}
 	} while (pm->pmSig == cpu_to_be16(HFS_NEW_PMAP_MAGIC));
 
 	return -ENOENT;
@@ -124,15 +128,15 @@ static int hfs_parse_new_pmap(struct super_block *sb, struct new_pmap *pm,
 int hfs_part_find(struct super_block *sb,
 		sector_t *part_start, sector_t *part_size)
 {
-	void *data;
+	void *buf, *data;
 	int res;
 
-	data = kmalloc(HFSPLUS_SECTOR_SIZE, GFP_KERNEL);
-	if (!data)
+	buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL);
+	if (!buf)
 		return -ENOMEM;
 
-	res = hfsplus_submit_bio(sb->s_bdev, *part_start + HFS_PMAP_BLK,
-				 data, READ);
+	res = hfsplus_submit_bio(sb, *part_start + HFS_PMAP_BLK,
+				 buf, &data, READ);
 	if (res)
 		goto out;
 
@@ -141,13 +145,13 @@ int hfs_part_find(struct super_block *sb,
 		res = hfs_parse_old_pmap(sb, data, part_start, part_size);
 		break;
 	case HFS_NEW_PMAP_MAGIC:
-		res = hfs_parse_new_pmap(sb, data, part_start, part_size);
+		res = hfs_parse_new_pmap(sb, buf, data, part_start, part_size);
 		break;
 	default:
 		res = -ENOENT;
 		break;
 	}
 out:
-	kfree(data);
+	kfree(buf);
 	return res;
 }
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 84a47b7..ab4857b 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -197,17 +197,17 @@ int hfsplus_sync_fs(struct super_block *sb, int wait)
 		write_backup = 1;
 	}
 
-	error2 = hfsplus_submit_bio(sb->s_bdev,
+	error2 = hfsplus_submit_bio(sb,
 				   sbi->part_start + HFSPLUS_VOLHEAD_SECTOR,
-				   sbi->s_vhdr, WRITE_SYNC);
+				   sbi->s_vhdr_buf, NULL, WRITE_SYNC);
 	if (!error)
 		error = error2;
 	if (!write_backup)
 		goto out;
 
-	error2 = hfsplus_submit_bio(sb->s_bdev,
+	error2 = hfsplus_submit_bio(sb,
 				  sbi->part_start + sbi->sect_count - 2,
-				  sbi->s_backup_vhdr, WRITE_SYNC);
+				  sbi->s_backup_vhdr_buf, NULL, WRITE_SYNC);
 	if (!error)
 		error2 = error;
 out:
@@ -251,8 +251,8 @@ static void hfsplus_put_super(struct super_block *sb)
 	hfs_btree_close(sbi->ext_tree);
 	iput(sbi->alloc_file);
 	iput(sbi->hidden_dir);
-	kfree(sbi->s_vhdr);
-	kfree(sbi->s_backup_vhdr);
+	kfree(sbi->s_vhdr_buf);
+	kfree(sbi->s_backup_vhdr_buf);
 	unload_nls(sbi->nls);
 	kfree(sb->s_fs_info);
 	sb->s_fs_info = NULL;
diff --git a/fs/hfsplus/wrapper.c b/fs/hfsplus/wrapper.c
index 4ac88ff..e3881a1 100644
--- a/fs/hfsplus/wrapper.c
+++ b/fs/hfsplus/wrapper.c
@@ -31,25 +31,67 @@ static void hfsplus_end_io_sync(struct bio *bio, int err)
 	complete(bio->bi_private);
 }
 
-int hfsplus_submit_bio(struct block_device *bdev, sector_t sector,
-		void *data, int rw)
+/*
+ * hfsplus_submit_bio - Perfrom block I/O
+ * @sb: super block of volume for I/O
+ * @sector: block to read or write, for blocks of HFSPLUS_SECTOR_SIZE bytes
+ * @buf: buffer for I/O
+ * @data: output pointer for location of requested data
+ * @rw: direction of I/O
+ *
+ * The unit of I/O is hfsplus_min_io_size(sb), which may be bigger than
+ * HFSPLUS_SECTOR_SIZE, and @buf must be sized accordingly. On reads
+ * @data will return a pointer to the start of the requested sector,
+ * which may not be the same location as @buf.
+ *
+ * If @sector is not aligned to the bdev logical block size it will
+ * be rounded down. For writes this means that @buf should contain data
+ * that starts at the rounded-down address. As long as the data was
+ * read using hfsplus_submit_bio() and the same buffer is used things
+ * will work correctly.
+ */
+int hfsplus_submit_bio(struct super_block *sb, sector_t sector,
+		void *buf, void **data, int rw)
 {
 	DECLARE_COMPLETION_ONSTACK(wait);
 	struct bio *bio;
 	int ret = 0;
+	unsigned int io_size;
+	loff_t start;
+	int offset;
+
+	/*
+	 * Align sector to hardware sector size and find offset. We
+	 * assume that io_size is a power of two, which _should_
+	 * be true.
+	 */
+	io_size = hfsplus_min_io_size(sb);
+	start = (loff_t)sector << HFSPLUS_SECTOR_SHIFT;
+	offset = start & (io_size - 1);
+	sector &= ~((io_size >> HFSPLUS_SECTOR_SHIFT) - 1);
 
 	bio = bio_alloc(GFP_NOIO, 1);
 	bio->bi_sector = sector;
-	bio->bi_bdev = bdev;
+	bio->bi_bdev = sb->s_bdev;
 	bio->bi_end_io = hfsplus_end_io_sync;
 	bio->bi_private = &wait;
 
-	/*
-	 * We always submit one sector at a time, so bio_add_page must not fail.
-	 */
-	if (bio_add_page(bio, virt_to_page(data), HFSPLUS_SECTOR_SIZE,
-			 offset_in_page(data)) != HFSPLUS_SECTOR_SIZE)
-		BUG();
+	if (!(rw & WRITE) && data)
+		*data = (u8 *)buf + offset;
+
+	while (io_size > 0) {
+		unsigned int page_offset = offset_in_page(buf);
+		unsigned int len = min_t(unsigned int, PAGE_SIZE - page_offset,
+					 io_size);
+
+		ret = bio_add_page(bio, virt_to_page(buf), len, page_offset);
+		if (ret != len) {
+			ret = -EIO;
+			goto out;
+		}
+		io_size -= len;
+		buf = (u8 *)buf + len;
+	}
 
 	submit_bio(rw, bio);
 	wait_for_completion(&wait);
@@ -57,8 +99,9 @@ int hfsplus_submit_bio(struct block_device *bdev, sector_t sector,
 	if (!bio_flagged(bio, BIO_UPTODATE))
 		ret = -EIO;
 
+out:
 	bio_put(bio);
-	return ret;
+	return ret < 0 ? ret : 0;
 }
 
 static int hfsplus_read_mdb(void *bufptr, struct hfsplus_wd *wd)
@@ -147,17 +190,17 @@ int hfsplus_read_wrapper(struct super_block *sb)
 	}
 
 	error = -ENOMEM;
-	sbi->s_vhdr = kmalloc(HFSPLUS_SECTOR_SIZE, GFP_KERNEL);
-	if (!sbi->s_vhdr)
+	sbi->s_vhdr_buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL);
+	if (!sbi->s_vhdr_buf)
 		goto out;
-	sbi->s_backup_vhdr = kmalloc(HFSPLUS_SECTOR_SIZE, GFP_KERNEL);
-	if (!sbi->s_backup_vhdr)
+	sbi->s_backup_vhdr_buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL);
+	if (!sbi->s_backup_vhdr_buf)
 		goto out_free_vhdr;
 
 reread:
-	error = hfsplus_submit_bio(sb->s_bdev,
-				   part_start + HFSPLUS_VOLHEAD_SECTOR,
-				   sbi->s_vhdr, READ);
+	error = hfsplus_submit_bio(sb, part_start + HFSPLUS_VOLHEAD_SECTOR,
+				   sbi->s_vhdr_buf, (void **)&sbi->s_vhdr,
+				   READ);
 	if (error)
 		goto out_free_backup_vhdr;
 
@@ -186,9 +229,9 @@ reread:
 		goto reread;
 	}
 
-	error = hfsplus_submit_bio(sb->s_bdev,
-				   part_start + part_size - 2,
-				   sbi->s_backup_vhdr, READ);
+	error = hfsplus_submit_bio(sb, part_start + part_size - 2,
+				   sbi->s_backup_vhdr_buf,
+				   (void **)&sbi->s_backup_vhdr, READ);
 	if (error)
 		goto out_free_backup_vhdr;
 
-- 
1.7.4.1

Re: Problems with hfsplus on ipods in 2.6.38+

[Christoph Hellwig]

On Mon, Jul 18, 2011 at 08:36:51AM -0500, Seth Forshee wrote:
> On Fri, Jul 15, 2011 at 09:39:00AM -0500, Seth Forshee wrote:
> > What I also see are GPFs in memory allocation code, which is what I
> > believe others have seen with my patch, and so far I haven't seen those
> > with the reversions. So I'm suspecting memory corruption, but I don't
> > yet see where the corruption is coming form. I found one problem, but I
> > don't suspect it's responsible for the GPFs.
> 
> I had some torture tests running over the weekend with the "one
> problem" mentioned above fixed, which was a couple of kfrees that were
> still using the old vhdr pointers. There were no oopses over a couple of
> days of testing. I've posted a new build on the bug in launchpad in
> hopes of getting some testing from those who were getting oopses quite
> readily. The updated patch follows.

Thanks a lot Seth.  I'll put it into my queue for Linux 3.1, and if
everthing works out fine we can push it out to -stable.

Re: Problems with hfsplus on ipods in 2.6.38+

[Daniel Barkalow]

On Mon, 18 Jul 2011, Seth Forshee wrote:

> On Fri, Jul 15, 2011 at 09:39:00AM -0500, Seth Forshee wrote:
> > What I also see are GPFs in memory allocation code, which is what I
> > believe others have seen with my patch, and so far I haven't seen those
> > with the reversions. So I'm suspecting memory corruption, but I don't
> > yet see where the corruption is coming form. I found one problem, but I
> > don't suspect it's responsible for the GPFs.
> 
> I had some torture tests running over the weekend with the "one
> problem" mentioned above fixed, which was a couple of kfrees that were
> still using the old vhdr pointers. There were no oopses over a couple of
> days of testing. I've posted a new build on the bug in launchpad in
> hopes of getting some testing from those who were getting oopses quite
> readily. The updated patch follows.

I should be able to try it this evening.

	-Daniel
*This .sig left intentionally blank*

Re: Problems with hfsplus on ipods in 2.6.38+

[Daniel Barkalow]

On Mon, 18 Jul 2011, Daniel Barkalow wrote:

> On Mon, 18 Jul 2011, Seth Forshee wrote:
> 
> > On Fri, Jul 15, 2011 at 09:39:00AM -0500, Seth Forshee wrote:
> > > What I also see are GPFs in memory allocation code, which is what I
> > > believe others have seen with my patch, and so far I haven't seen those
> > > with the reversions. So I'm suspecting memory corruption, but I don't
> > > yet see where the corruption is coming form. I found one problem, but I
> > > don't suspect it's responsible for the GPFs.
> > 
> > I had some torture tests running over the weekend with the "one
> > problem" mentioned above fixed, which was a couple of kfrees that were
> > still using the old vhdr pointers. There were no oopses over a couple of
> > days of testing. I've posted a new build on the bug in launchpad in
> > hopes of getting some testing from those who were getting oopses quite
> > readily. The updated patch follows.
> 
> I should be able to try it this evening.

It seems to work. I connected and disconnected it a bunch of times (due to 
a somewhat flaky connector) and mounted it, wrote to it, had it disconnect 
itself, and mounted it and ejected it, and generated some memory load, and 
got no oopses. (Just to be clear, I was testing the 0001 and 0003 of the 
first attempt with the formerly-attached patch as 0002, replacing the 
original 0002.)

Tested-by: Daniel Barkalow <barkalow@iabervon.org>

	-Daniel
*This .sig left intentionally blank*