lkml.org 
[lkml]   [2010]   [Mar]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: RFC: direct MTD support for SquashFS
    From
    On Thu, Mar 18, 2010 at 4:38 PM, Ferenc Wagner <wferi@niif.hu> wrote:
    >
    > I could only compare apples to oranges before porting the patch to the
    > LZMA variant.  So I refrain from that for a couple of days yet.  But
    > meanwhile I started adding a pluggable backend framework to SquashFS,
    > and would much appreciate some comments about the applicability of this
    > idea.  The patch is (intended to be) a no-op, applies on top of current
    > git (a3d3203e4bb40f253b1541e310dc0f9305be7c84).

    This looks promising, making the backend pluggable (like the new
    compressor framework) is far better and cleaner than scattering the
    code full of #ifdef's. Far better than the previous patch :-)

    A couple of specific comments...

    +/* A backend is initialized for each SquashFS block read operation,
    + * making further sequential reads possible from the block.
    + */
    +static void *bdev_init(struct squashfs_sb_info *msblk, u64 index,
    size_t length)
    +{
    + struct squashfs_bdev *bdev = msblk->backend_data;
    + struct buffer_head *bh;
    +
    + bh = kcalloc((msblk->block_size >> bdev->devblksize_log2) + 1,
    + sizeof(*bh), GFP_KERNEL);

    You should alloc against the larger of msblk->block_size and
    METADATA_SIZE (8 Kbytes). Block_size could be 4 Kbytes only.

    +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?
    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...

    Phillip
    --
    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/

    \
     
     \ /
      Last update: 2010-03-18 22:43    [W:0.023 / U:30.240 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site