Re: Kernel 3.1.0-rc4 oops when connecting iPod

[Pavel Ivanov]

On Tue, Sep 6, 2011 at 11:09 AM, Pavel Ivanov <paivanof@xxxxxxxxx> wrote:
>>> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
>>> index c106ca2..5458be4 100644
>>> --- a/fs/hfsplus/super.c
>>> +++ b/fs/hfsplus/super.c
>>> @@ -345,6 +345,8 @@ static int hfsplus_fill_super(struct
>>> super_block
>>> *sb, void *data, int silent)
>>>      struct qstr str;
>>>      struct nls_table *nls = NULL;
>>>      int err;
>>> +    unsigned check_blksz_bits;
>>> +    u64 check_num_blocks;
>>>
>>>      err = -EINVAL;
>>>      sbi = kzalloc(sizeof(*sbi),
>>> GFP_KERNEL);
>>> @@ -399,10 +401,15 @@ static int hfsplus_fill_super(struct
>>> super_block
>>> *sb, void *data, int silent)
>>>      if (!sbi->rsrc_clump_blocks)
>>>
>>> sbi->rsrc_clump_blocks = 1;
>>>
>>> -    err =
>>> generic_check_addressable(sbi->alloc_blksz_shift,
>>> -
>>>
>>> sbi->total_blocks);
>>> +    check_blksz_bits =
>>> sbi->alloc_blksz_shift;
>>> +    check_num_blocks =
>>> sbi->total_blocks;
>>> +    if (sbi->fs_shift != 0) {
>>> +        check_num_blocks
>>> <<= sbi->fs_shift;
>>> +        check_blksz_bits -=
>>> sbi->fs_shift;
>>> +    }
>>> +    err =
>>> generic_check_addressable(check_blksz_bits,
>>> check_num_blocks);
>>>      if (err) {
>>>         printk(KERN_ERR "hfs:
>>> filesystem size too large.\n");
>>>          goto out_free_vhdr;
>>
>> This is a bit ugly - what it does is massage the two values taking into account of sbi->fs_shift to pass check_addressable() . I think either check_addressable should be extended, or this be abstracted out say, to check_hfs_addressable() (with __inline__ if desired) to be cleaner.
>
> Agreed. It was more of a quick hack to prove the correctness of such
> change direction. I'll think how to do it better.

Hin-Tak,

How about the following patch?


Subject: [PATCH] hfsplus: Allow to mount filesystem with block size
greater than PAGE_SIZE

Commit c6d5f5fa (hfsplus: lift the 2TB size limit) added call to
generic_check_addressable() but used the system's internal block size
which can be larger than PAGE_SIZE and will cause
generic_check_addressable() to return -EINVAL in this case. This
results in impossibility to mount file systems with such block sizes.
To fix it we have to use block size understandable by
generic_check_addressable() and adjust number of blocks accordingly.

Signed-off-by: Pavel Ivanov <paivanof@xxxxxxxxx>
Cc: <stable@xxxxxxxxxx>
---

diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index c106ca2..26ac5a5 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -399,8 +399,17 @@ static int hfsplus_fill_super(struct super_block
*sb, void *data, int silent)
 	if (!sbi->rsrc_clump_blocks)
 		sbi->rsrc_clump_blocks = 1;

-	err = generic_check_addressable(sbi->alloc_blksz_shift,
-					sbi->total_blocks);
+	/*
+	 * Check that filesystem data can be addressed by sector_t. Obvious way
+	 * to do that is to call generic_check_addressable with alloc_blksz_shift
+	 * and total_blocks as parameters. But alloc_blksz_shift can be greater
+	 * than PAGE_SIZE_SHIFT and in this case it won't be accepted by
+	 * generic_check_addressable. s_blocksize_bits will always be accepted
+	 * and difference between it and alloc_blksz_shift will be always in
+	 * fs_shift.
+	 */
+	err = generic_check_addressable(sb->s_blocksize_bits,
+			(u64)sbi->total_blocks << sbi->fs_shift);
 	if (err) {
 		printk(KERN_ERR "hfs: filesystem size too large.\n");
 		goto out_free_vhdr;
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/