Comments

Phillip Lougher <phillip.lougher@gmail.com> writes:
> 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.
I plugged in a max(). Couldn't that trailing +1 be converted into a +2
like this?
bh = kcalloc((max(msblk->block_size, METADATA_SIZE) + 2) >> bdev->devblksize_log2
> +static int fill_bdev_super(struct super_block *sb, void *data, int silent)>> 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...
I solved it by introducing a callback function for adding the backend.
That may be overkill, but it seems to give the most shared code.
The attached patch series survived some testing here. My only doubt:
the current backend interface necessitates a memory copy from the buffer
heads. This is no problem for mtd and lzma which copy the data anyway,
but makes this code less efficient in the bdev+zlib case.
I've got one more patch, which I forgot to export, to pull out the
common logic from the backend init functions back into squashfs_read_data().
With the bdev backend, that entails reading the first block twice in a
row most of the time. This again could be worked around by extending
the backend interface, but I'm not sure if it's worth it.
How does this look like now?