Signed-off-by: Phillip Lougher <[email protected]>
---
fs/squashfs/Makefile | 2 +-
fs/squashfs/block.c | 74 ++----------------------------
fs/squashfs/squashfs.h | 4 ++
fs/squashfs/zlib_wrapper.c | 109 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 118 insertions(+), 71 deletions(-)
create mode 100644 fs/squashfs/zlib_wrapper.c
diff --git a/fs/squashfs/Makefile b/fs/squashfs/Makefile
index 70e3244..a397e6f 100644
--- a/fs/squashfs/Makefile
+++ b/fs/squashfs/Makefile
@@ -4,4 +4,4 @@
obj-$(CONFIG_SQUASHFS) += squashfs.o
squashfs-y += block.o cache.o dir.o export.o file.o fragment.o id.o inode.o
-squashfs-y += namei.o super.o symlink.o
+squashfs-y += namei.o super.o symlink.o zlib_wrapper.o
diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c
index 2a79603..5cd3934 100644
--- a/fs/squashfs/block.c
+++ b/fs/squashfs/block.c
@@ -29,7 +29,6 @@
#include <linux/fs.h>
#include <linux/vfs.h>
#include <linux/slab.h>
-#include <linux/mutex.h>
#include <linux/string.h>
#include <linux/buffer_head.h>
#include <linux/zlib.h>
@@ -153,72 +152,10 @@ int squashfs_read_data(struct super_block *sb, void **buffer, u64 index,
}
if (compressed) {
- int zlib_err = 0, zlib_init = 0;
-
- /*
- * Uncompress block.
- */
-
- mutex_lock(&msblk->read_data_mutex);
-
- msblk->stream.avail_out = 0;
- msblk->stream.avail_in = 0;
-
- bytes = length;
- do {
- if (msblk->stream.avail_in == 0 && k < b) {
- avail = min(bytes, msblk->devblksize - offset);
- bytes -= avail;
- wait_on_buffer(bh[k]);
- if (!buffer_uptodate(bh[k]))
- goto release_mutex;
-
- if (avail == 0) {
- offset = 0;
- put_bh(bh[k++]);
- continue;
- }
-
- msblk->stream.next_in = bh[k]->b_data + offset;
- msblk->stream.avail_in = avail;
- offset = 0;
- }
-
- if (msblk->stream.avail_out == 0 && page < pages) {
- msblk->stream.next_out = buffer[page++];
- msblk->stream.avail_out = PAGE_CACHE_SIZE;
- }
-
- if (!zlib_init) {
- zlib_err = zlib_inflateInit(&msblk->stream);
- if (zlib_err != Z_OK) {
- ERROR("zlib_inflateInit returned"
- " unexpected result 0x%x,"
- " srclength %d\n", zlib_err,
- srclength);
- goto release_mutex;
- }
- zlib_init = 1;
- }
-
- zlib_err = zlib_inflate(&msblk->stream, Z_SYNC_FLUSH);
-
- if (msblk->stream.avail_in == 0 && k < b)
- put_bh(bh[k++]);
- } while (zlib_err == Z_OK);
-
- if (zlib_err != Z_STREAM_END) {
- ERROR("zlib_inflate error, data probably corrupt\n");
- goto release_mutex;
- }
-
- zlib_err = zlib_inflateEnd(&msblk->stream);
- if (zlib_err != Z_OK) {
- ERROR("zlib_inflate error, data probably corrupt\n");
- goto release_mutex;
- }
- length = msblk->stream.total_out;
- mutex_unlock(&msblk->read_data_mutex);
+ length = zlib_uncompress(msblk, buffer, bh, b, offset, length,
+ srclength, pages);
+ if (length < 0)
+ goto read_failure;
} else {
/*
* Block is uncompressed.
@@ -255,9 +192,6 @@ int squashfs_read_data(struct super_block *sb, void **buffer, u64 index,
kfree(bh);
return length;
-release_mutex:
- mutex_unlock(&msblk->read_data_mutex);
-
block_release:
for (; k < b; k++)
put_bh(bh[k]);
diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
index 0e9feb6..988bdce 100644
--- a/fs/squashfs/squashfs.h
+++ b/fs/squashfs/squashfs.h
@@ -70,6 +70,10 @@ extern struct inode *squashfs_iget(struct super_block *, long long,
unsigned int);
extern int squashfs_read_inode(struct inode *, long long);
+/* zlib_wrapper.c */
+extern int zlib_uncompress(struct squashfs_sb_info *, void **,
+ struct buffer_head **, int, int, int, int, int);
+
/*
* Inodes and files operations
*/
diff --git a/fs/squashfs/zlib_wrapper.c b/fs/squashfs/zlib_wrapper.c
new file mode 100644
index 0000000..486a2a7
--- /dev/null
+++ b/fs/squashfs/zlib_wrapper.c
@@ -0,0 +1,109 @@
+/*
+ * Squashfs - a compressed read only filesystem for Linux
+ *
+ * Copyright (c) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009
+ * Phillip Lougher <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2,
+ * or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * zlib_wrapper.c
+ */
+
+
+#include <linux/mutex.h>
+#include <linux/buffer_head.h>
+#include <linux/zlib.h>
+
+#include "squashfs_fs.h"
+#include "squashfs_fs_sb.h"
+#include "squashfs_fs_i.h"
+#include "squashfs.h"
+
+int zlib_uncompress(struct squashfs_sb_info *msblk, void **buffer,
+ struct buffer_head **bh, int b, int offset, int length, int srclength,
+ int pages)
+{
+ int zlib_err = 0, zlib_init = 0;
+ int avail, bytes, k = 0, page = 0;
+
+ mutex_lock(&msblk->read_data_mutex);
+
+ msblk->stream.avail_out = 0;
+ msblk->stream.avail_in = 0;
+
+ bytes = length;
+ do {
+ if (msblk->stream.avail_in == 0 && k < b) {
+ avail = min(bytes, msblk->devblksize - offset);
+ bytes -= avail;
+ wait_on_buffer(bh[k]);
+ if (!buffer_uptodate(bh[k]))
+ goto release_mutex;
+
+ if (avail == 0) {
+ offset = 0;
+ put_bh(bh[k++]);
+ continue;
+ }
+
+ msblk->stream.next_in = bh[k]->b_data + offset;
+ msblk->stream.avail_in = avail;
+ offset = 0;
+ }
+
+ if (msblk->stream.avail_out == 0 && page < pages) {
+ msblk->stream.next_out = buffer[page++];
+ msblk->stream.avail_out = PAGE_CACHE_SIZE;
+ }
+
+ if (!zlib_init) {
+ zlib_err = zlib_inflateInit(&msblk->stream);
+ if (zlib_err != Z_OK) {
+ ERROR("zlib_inflateInit returned unexpected "
+ "result 0x%x, srclength %d\n",
+ zlib_err, srclength);
+ goto release_mutex;
+ }
+ zlib_init = 1;
+ }
+
+ zlib_err = zlib_inflate(&msblk->stream, Z_SYNC_FLUSH);
+
+ if (msblk->stream.avail_in == 0 && k < b)
+ put_bh(bh[k++]);
+ } while (zlib_err == Z_OK);
+
+ if (zlib_err != Z_STREAM_END) {
+ ERROR("zlib_inflate error, data probably corrupt\n");
+ goto release_mutex;
+ }
+
+ zlib_err = zlib_inflateEnd(&msblk->stream);
+ if (zlib_err != Z_OK) {
+ ERROR("zlib_inflate error, data probably corrupt\n");
+ goto release_mutex;
+ }
+
+ mutex_unlock(&msblk->read_data_mutex);
+ return msblk->stream.total_out;
+
+release_mutex:
+ mutex_unlock(&msblk->read_data_mutex);
+
+ for (; k < b; k++)
+ put_bh(bh[k]);
+
+ return -EIO;
+}
--
1.6.3.3
On Mon, 07 Dec 2009 02:25:08 +0000
Phillip Lougher <[email protected]> wrote:
> +++ b/fs/squashfs/zlib_wrapper.c
> @@ -0,0 +1,109 @@
> +/*
> + * Squashfs - a compressed read only filesystem for Linux
> + *
> + * Copyright (c) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009
> + * Phillip Lougher <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2,
> + * or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
> + *
> + * zlib_wrapper.c
> + */
> +
> +
> +#include <linux/mutex.h>
> +#include <linux/buffer_head.h>
> +#include <linux/zlib.h>
> +
> +#include "squashfs_fs.h"
> +#include "squashfs_fs_sb.h"
> +#include "squashfs_fs_i.h"
> +#include "squashfs.h"
> +
> +int zlib_uncompress(struct squashfs_sb_info *msblk, void **buffer,
> + struct buffer_head **bh, int b, int offset, int length, int srclength,
> + int pages)
This isn't a very good function name. zlib_uncompress() now becomes a
kernel-wide identifier, but it's part of squashfs, not part of zlib.
Maybe that gets fixed in a later patch. If so, thwap me.
Andrew Morton wrote:
> On Mon, 07 Dec 2009 02:25:08 +0000
> Phillip Lougher <[email protected]> wrote:
>
>> +
>> +int zlib_uncompress(struct squashfs_sb_info *msblk, void **buffer,
>> + struct buffer_head **bh, int b, int offset, int length, int srclength,
>> + int pages)
>
> This isn't a very good function name. zlib_uncompress() now becomes a
> kernel-wide identifier, but it's part of squashfs, not part of zlib.
>
> Maybe that gets fixed in a later patch. If so, thwap me.
>
Yes, they get fixed up in [PATCH 3/9] Squashfs: add a decompressor framework.
That patch makes the functions static, and instead exports them via a
suitably named decompressor ops structure.
+const struct squashfs_decompressor squashfs_zlib_comp_ops = {
+ .init = zlib_init,
+ .free = zlib_free,
+ .decompress = zlib_uncompress,
+ .id = ZLIB_COMPRESSION,
+ .name = "zlib",
+ .supported = 1
+};
I split the patches up to make them easier to review. The first two patches
move the zlib code out to a separate file (ready for adding the framework).
The third patch adds the framework. At the time of the second patch, however,
to not break compilation, the functions have to be global. In hindsight
I should have made named the functions squashfs_xxx, and removed the squashfs_
when they were made static in the third patch.
Thanks
Phillip
On Tue, 2009-12-08 at 22:20 +0000, Phillip Lougher wrote:
> Andrew Morton wrote:
> > On Mon, 07 Dec 2009 02:25:08 +0000
> > Phillip Lougher <[email protected]> wrote:
> >
> >> +
> >> +int zlib_uncompress(struct squashfs_sb_info *msblk, void **buffer,
> >> + struct buffer_head **bh, int b, int offset, int length, int srclength,
> >> + int pages)
> >
> > This isn't a very good function name. zlib_uncompress() now becomes a
> > kernel-wide identifier, but it's part of squashfs, not part of zlib.
> >
> > Maybe that gets fixed in a later patch. If so, thwap me.
> >
>
> Yes, they get fixed up in [PATCH 3/9] Squashfs: add a decompressor framework.
> That patch makes the functions static, and instead exports them via a
> suitably named decompressor ops structure.
>
> +const struct squashfs_decompressor squashfs_zlib_comp_ops = {
> + .init = zlib_init,
> + .free = zlib_free,
> + .decompress = zlib_uncompress,
> + .id = ZLIB_COMPRESSION,
> + .name = "zlib",
> + .supported = 1
> +};
>
> I split the patches up to make them easier to review. The first two patches
> move the zlib code out to a separate file (ready for adding the framework).
> The third patch adds the framework. At the time of the second patch, however,
> to not break compilation, the functions have to be global. In hindsight
> I should have made named the functions squashfs_xxx, and removed the squashfs_
> when they were made static in the third patch.
Did you consider using cryptoapi? UBIFS uses zlib/lzo in cryptoapi - it
is a very clean way.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
Artem Bityutskiy wrote:
>
> Did you consider using cryptoapi? UBIFS uses zlib/lzo in cryptoapi - it
> is a very clean way.
>
Yes I did consider using the cryptoapi, but this doesn't have support for
lzma in mainline.
My aim is add lzma filesystem support to Squashfs using the existing kernel
lzma iplementation, and to make as few changes to that as necessary.
Phillip
Hi Phillip,
On Thu, Dec 10, 2009 at 01:38, Phillip Lougher
<[email protected]> wrote:
> Artem Bityutskiy wrote:
>>
>> Did you consider using cryptoapi? UBIFS uses zlib/lzo in cryptoapi - it
>> is a very clean way.
Exactly my question, as that's why the Crypto API was extended with
support for partial (de)compression in the first place ;-)
In addition, using the Crypto API has the advantage that Squashfs will
use hardware decompression if/when available.
> Yes I did consider using the cryptoapi, but this doesn't have support for
> lzma in mainline.
IIRC, Felix Fietkau added support for that for OpenWRT...
> My aim is add lzma filesystem support to Squashfs using the existing kernel
> lzma iplementation, and to make as few changes to that as necessary.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Geert Uytterhoeven wrote:
>
>> Yes I did consider using the cryptoapi, but this doesn't have support for
>> lzma in mainline.
>
> IIRC, Felix Fietkau added support for that for OpenWRT...
>
Yes, but it isn't in mainline, and OpenWRT don't appear to have tried to submit
it. IMHO the major problem with their patch is it uses a second private copy
of lzma, rather than being a wrapper around the pre-existing lzma implementation.
I don't think it's going to be easy to get a second lzma implementation accepted
into mainline, when it took so long to get one accepted. I have nothing against
the cryptoapi, but it doesn't seem likely to be getting lzma support anytime soon.
As I previously said my aim is to use the pre-existing lzma implementation. Unless
it is stupendously bad (which it isn't), that seems to be the quickest, easiest and
best way to get lzma support into Squashfs.
Phillip
On 2009-12-10 10:17 PM, Phillip Lougher wrote:
> Geert Uytterhoeven wrote:
>>
>>> Yes I did consider using the cryptoapi, but this doesn't have support for
>>> lzma in mainline.
>>
>> IIRC, Felix Fietkau added support for that for OpenWRT...
>>
>
> Yes, but it isn't in mainline, and OpenWRT don't appear to have tried to submit
> it. IMHO the major problem with their patch is it uses a second private copy
> of lzma, rather than being a wrapper around the pre-existing lzma implementation.
> I don't think it's going to be easy to get a second lzma implementation accepted
> into mainline, when it took so long to get one accepted. I have nothing against
> the cryptoapi, but it doesn't seem likely to be getting lzma support anytime soon.
>
> As I previously said my aim is to use the pre-existing lzma implementation. Unless
> it is stupendously bad (which it isn't), that seems to be the quickest, easiest and
> best way to get lzma support into Squashfs.
The main reason why my implementation couldn't be done as a simple
wrapper around the existing implementation is that it reuses the
caller's split output buffers instead wasting precious RAM by allocating
a contiguous buffer big enough to cover the entire block.
Especially at bigger block sizes (which provide better compression),
this probably has noticeable effects on tiny embedded systems with
little RAM.
- Felix
Geert Uytterhoeven wrote:
> Hi Phillip,
>
> On Thu, Dec 10, 2009 at 01:38, Phillip Lougher
> <[email protected]> wrote:
>> Artem Bityutskiy wrote:
>>> Did you consider using cryptoapi? UBIFS uses zlib/lzo in cryptoapi - it
>>> is a very clean way.
>
> Exactly my question, as that's why the Crypto API was extended with
> support for partial (de)compression in the first place ;-)
>
Your cryptoapi patches are on my "must do something about them" list, I've
not forgotten them :-) The lack of lzma in the crypotapi made moving
solely to the cryptoapi difficult, when I wanted to add lzma decompression
support.
The decompressor framework is my _solution_ to this problem. This allows
cryptoapi support to be added as additional decompression_wrapper, with
multiple compression types using the crptoapi wrapper as appropriate.
This has the advantage it allows Squashfs to use the cryptoapi (with
its advantages), without needlessly restricting Squashfs to the
compression algorithms supported by the cryptoapi.
Let's get this mainlined, and then we can work on getting your
cryptoapi patches integrated into that. That'll make me happy,
and hopefully you'll be happy too?
Thanks
Phillip