linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 1/9] Squashfs: move zlib decompression wrapper code into a separate file
       [not found] <E1NHTHg-0001sM-NQ@dylan.lougher.demon.co.uk>
@ 2009-12-07 22:57 ` Andrew Morton
  2009-12-08 22:20   ` Phillip Lougher
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2009-12-07 22:57 UTC (permalink / raw)
  To: Phillip Lougher
  Cc: linux-embedded, linux-fsdevel, linux-kernel, phillip.lougher, tim.bird

On Mon, 07 Dec 2009 02:25:08 +0000
Phillip Lougher <phillip@lougher.demon.co.uk> 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 <phillip@lougher.demon.co.uk>
> + *
> + * 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/9] Squashfs: move zlib decompression wrapper code into a separate file
  2009-12-07 22:57 ` [PATCH 1/9] Squashfs: move zlib decompression wrapper code into a separate file Andrew Morton
@ 2009-12-08 22:20   ` Phillip Lougher
  2009-12-09 10:00     ` Artem Bityutskiy
  0 siblings, 1 reply; 10+ messages in thread
From: Phillip Lougher @ 2009-12-08 22:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-embedded, linux-fsdevel, linux-kernel, phillip.lougher, tim.bird

Andrew Morton wrote:
> On Mon, 07 Dec 2009 02:25:08 +0000
> Phillip Lougher <phillip@lougher.demon.co.uk> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/9] Squashfs: move zlib decompression wrapper code into a separate file
  2009-12-08 22:20   ` Phillip Lougher
@ 2009-12-09 10:00     ` Artem Bityutskiy
  2009-12-10  0:38       ` Phillip Lougher
  0 siblings, 1 reply; 10+ messages in thread
From: Artem Bityutskiy @ 2009-12-09 10:00 UTC (permalink / raw)
  To: Phillip Lougher
  Cc: Andrew Morton, linux-embedded, linux-fsdevel, linux-kernel,
	phillip.lougher, tim.bird

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 <phillip@lougher.demon.co.uk> 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 (Артём Битюцкий)


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/9] Squashfs: move zlib decompression wrapper code into a separate file
  2009-12-09 10:00     ` Artem Bityutskiy
@ 2009-12-10  0:38       ` Phillip Lougher
  2009-12-10 10:01         ` Geert Uytterhoeven
  0 siblings, 1 reply; 10+ messages in thread
From: Phillip Lougher @ 2009-12-10  0:38 UTC (permalink / raw)
  To: dedekind1
  Cc: Andrew Morton, linux-embedded, linux-fsdevel, linux-kernel,
	phillip.lougher, tim.bird

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/9] Squashfs: move zlib decompression wrapper code into a  separate file
  2009-12-10  0:38       ` Phillip Lougher
@ 2009-12-10 10:01         ` Geert Uytterhoeven
  2009-12-10 21:17           ` Phillip Lougher
  2009-12-10 23:20           ` Phillip Lougher
  0 siblings, 2 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2009-12-10 10:01 UTC (permalink / raw)
  To: Phillip Lougher
  Cc: dedekind1, Andrew Morton, linux-embedded, linux-fsdevel,
	linux-kernel, phillip.lougher, Tim Bird, Geert Uytterhoeven,
	Felix Fietkau

Hi Phillip,

On Thu, Dec 10, 2009 at 01:38, Phillip Lougher
<phillip@lougher.demon.co.uk> 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 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/9] Squashfs: move zlib decompression wrapper code into a  separate file
  2009-12-10 10:01         ` Geert Uytterhoeven
@ 2009-12-10 21:17           ` Phillip Lougher
  2009-12-10 22:20             ` Felix Fietkau
  2009-12-10 23:20           ` Phillip Lougher
  1 sibling, 1 reply; 10+ messages in thread
From: Phillip Lougher @ 2009-12-10 21:17 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: dedekind1, Andrew Morton, linux-embedded, linux-fsdevel,
	linux-kernel, phillip.lougher, Tim Bird, Geert Uytterhoeven,
	Felix Fietkau

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/9] Squashfs: move zlib decompression wrapper code into a  separate file
  2009-12-10 21:17           ` Phillip Lougher
@ 2009-12-10 22:20             ` Felix Fietkau
  0 siblings, 0 replies; 10+ messages in thread
From: Felix Fietkau @ 2009-12-10 22:20 UTC (permalink / raw)
  To: Phillip Lougher
  Cc: Geert Uytterhoeven, dedekind1, Andrew Morton, linux-embedded,
	linux-fsdevel, linux-kernel, phillip.lougher, Tim Bird,
	Geert Uytterhoeven

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/9] Squashfs: move zlib decompression wrapper code into a  separate file
  2009-12-10 10:01         ` Geert Uytterhoeven
  2009-12-10 21:17           ` Phillip Lougher
@ 2009-12-10 23:20           ` Phillip Lougher
  1 sibling, 0 replies; 10+ messages in thread
From: Phillip Lougher @ 2009-12-10 23:20 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: dedekind1, Andrew Morton, linux-embedded, linux-fsdevel,
	linux-kernel, phillip.lougher, Tim Bird, Geert Uytterhoeven,
	Felix Fietkau

Geert Uytterhoeven wrote:
> Hi Phillip,
> 
> On Thu, Dec 10, 2009 at 01:38, Phillip Lougher
> <phillip@lougher.demon.co.uk> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/9] Squashfs: move zlib decompression wrapper code into a separate file
@ 2009-12-07  8:37 Phillip Lougher
  0 siblings, 0 replies; 10+ messages in thread
From: Phillip Lougher @ 2009-12-07  8:37 UTC (permalink / raw)
  To: akpm, linux-embedded, linux-fsdevel, linux-kernel,
	phillip.lougher, tim.bird


Signed-off-by: Phillip Lougher <phillip@lougher.demon.co.uk>
---
 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 <phillip@lougher.demon.co.uk>
+ *
+ * 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 1/9] Squashfs: move zlib decompression wrapper code into a separate file
@ 2009-12-07  8:25 root
  0 siblings, 0 replies; 10+ messages in thread
From: root @ 2009-12-07  8:25 UTC (permalink / raw)
  To: akpm, linux-embedded, linux-fsdevel, linux-kernel,
	phillip.lougher, tim.bird


Signed-off-by: Phillip Lougher <phillip@lougher.demon.co.uk>
---
 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 <phillip@lougher.demon.co.uk>
+ *
+ * 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-12-10 23:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <E1NHTHg-0001sM-NQ@dylan.lougher.demon.co.uk>
2009-12-07 22:57 ` [PATCH 1/9] Squashfs: move zlib decompression wrapper code into a separate file Andrew Morton
2009-12-08 22:20   ` Phillip Lougher
2009-12-09 10:00     ` Artem Bityutskiy
2009-12-10  0:38       ` Phillip Lougher
2009-12-10 10:01         ` Geert Uytterhoeven
2009-12-10 21:17           ` Phillip Lougher
2009-12-10 22:20             ` Felix Fietkau
2009-12-10 23:20           ` Phillip Lougher
2009-12-07  8:37 Phillip Lougher
  -- strict thread matches above, loose matches on Subject: below --
2009-12-07  8:25 root

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).