Re: RFC: direct MTD support for SquashFS

[Ferenc Wagner]

Phillip Lougher <phillip.lougher@xxxxxxxxx> writes:

> On Fri, Mar 19, 2010 at 1:05 AM, Ferenc Wagner <wferi@xxxxxxx> wrote:
>
>> Ferenc Wagner <wferi@xxxxxxx> writes:
>>
>>> Phillip Lougher <phillip.lougher@xxxxxxxxx> writes:
>>>
>>>> On Thu, Mar 18, 2010 at 4:38 PM, Ferenc Wagner <wferi@xxxxxxx> wrote:
>>>>
>>>> +static int fill_bdev_super(struct super_block *sb, void *data, int silent)
>>>> +{
>>>> + Â Âstruct squashfs_sb_info *msblk;
>>>> + Â Âstruct squashfs_bdev *bdev;
>>>> + Â Âint err = squashfs_fill_super2(sb, data, silent, &squashfs_bdev_ops);
>>>> + Â Âif (err)
>>>> + Â Â Â Â Â Âreturn err;
>>>> +
>>>> + Â Âbdev = kzalloc(sizeof(*bdev), GFP_KERNEL);
>>>> + Â Âif (!bdev)
>>>> + Â Â Â Â Â Âreturn -ENOMEM;
>>>> +
>>>> + Â Âbdev->devblksize = sb_min_blocksize(sb, BLOCK_SIZE);
>>>> + Â Âbdev->devblksize_log2 = ffz(~bdev->devblksize);
>>>> +
>>>> + Â Âmsblk = sb->s_fs_info;
>>>> + Â Âmsblk->backend_data = bdev;
>>>> + Â Âreturn 0;
>>>> +}
>>>>
>>>> This function looks rather 'back-to-front' to me. ÂI'm assuming that
>>>> squashfs_fill_super2() will be the current fill superblock function?
>>>
>>> Yes, with the extra parameter added.
>>>
>>>> This function wants to read data off the filesystem through the
>>>> backend, and yet the backend (bdev, mblk->backend_data) hasn't been
>>>> initialised when it's called...
>>>
>>> It can't be, because msblk = sb->s_fs_info is allocated by
>>> squashfs_fill_super(). ÂNow it will be passed the ops, so after
>>> allocating msblk it can also fill out the ops. ÂAfter that it can read,
>>> and squashfs_read_data() will call the init, read and free operations of
>>> the backend.
>>
>> And here we indeed have a rather fundamental problem. ÂThis isn't
>> specific to the discussed plugin system at all. ÂEven in the current
>> code, to set msblk->block_size squashfs_fill_super() calls
>> squashfs_read_table() to read the superblock, which in turn calls
>> squashfs_read_data(), which uses msblk->block_size to allocate enough
>> buffer heads, but msblk->block_size just can't be set at this point.
>> msblk->bytes_used is preset with a dummy value to make the read
>> possible, but msblk->block_size is not. ÂFortunately, one buffer head is
>> allocated each time nevertheless. ÂI wonder what a correct solution
>> would look like..
>
> Block_size is known to be zero (the structure has been zeroed out at
> alloc), and so it is known that the one block alloced in this case
> will be correct.

If block_size=0 is always a good dummy value for this single call,
that's great.  Fixing this in a general way in the backend framework
might require allocating and partly initializing squashfs_sb_info in the
backend specific fill_super() function, before calling squashfs_fill_super()
for finalizing it.  Even though it may be possible to work around this
for the bdev or mtd backends, it probably isn't worth it.

> Congratulations you've managed to really piss me off in your third or
> so email.

Sorry, I'm not sure I understand.  If you mean that I made an ass of
myself by my questions, that's OK, I'm certainly a newbie and I
admittedly have no idea what I'm fiddling with.  Please feel free to
ignore stupid questions.  On the other hand, if you mean that I hurt
your feelings in any way, I'd like to apologize: it certainly wasn't my
intention, but I might have chosen inappropriate terms.  Sorry for that.
-- 
Cheers,
Feri.
--
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/