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-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
