Messages in this thread | | | From | Ferenc Wagner <> | Subject | Re: RFC: direct MTD support for SquashFS | Date | Fri, 19 Mar 2010 15:12:44 +0100 |
| |
Phillip Lougher <phillip.lougher@gmail.com> writes:
> On Fri, Mar 19, 2010 at 1:05 AM, Ferenc Wagner <wferi@niif.hu> wrote: > >> Ferenc Wagner <wferi@niif.hu> writes: >> >>> Phillip Lougher <phillip.lougher@gmail.com> writes: >>> >>>> On Thu, Mar 18, 2010 at 4:38 PM, Ferenc Wagner <wferi@niif.hu> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |