Re: [PATCH 1/4] Squashfs: Refactor decompressor interface and code (V3)
On Thu, Nov 14, 2013 at 05:14:16AM +, Phillip Lougher wrote: > The decompressor interface and code was written from > the point of view of single-threaded operation. In doing > so it mixed a lot of single-threaded implementation specific > aspects into the decompressor code and elsewhere which makes it > difficult to seamlessly support multiple different decompressor > implementations. > > This patch does the following: > > 1. It removes compressor_options parsing from the decompressor > init() function. This allows the decompressor init() function > to be dynamically called to instantiate multiple decompressors, > without the compressor options needing to be read and parsed each > time. > > 2. It moves threading and all sleeping operations out of the > decompressors. In doing so, it makes the decompressors > non-blocking wrappers which only deal with interfacing with > the decompressor implementation. > > 3. It splits decompressor.[ch] into decompressor generic functions >in decompressor.[ch], and moves the single threaded >decompressor implementation into decompressor_single.c. > > The result of this patch is Squashfs should now be able to > support multiple decompressors by adding new decompressor_xxx.c > files with specialised implementations of the functions in > decompressor_single.c > > V3: > * decompressor_single.c: Remove kfree(comp_opts) in error path > * of squashfs_decompressor_create() > * Double free of comp_opts found by static analysis, reported by > * Dan Carpenter > > Signed-off-by: Phillip Lougher Reviewed-by: Minchan Kim -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] Squashfs: Refactor decompressor interface and code (V3)
On Thu, Nov 14, 2013 at 05:14:16AM +, Phillip Lougher wrote: The decompressor interface and code was written from the point of view of single-threaded operation. In doing so it mixed a lot of single-threaded implementation specific aspects into the decompressor code and elsewhere which makes it difficult to seamlessly support multiple different decompressor implementations. This patch does the following: 1. It removes compressor_options parsing from the decompressor init() function. This allows the decompressor init() function to be dynamically called to instantiate multiple decompressors, without the compressor options needing to be read and parsed each time. 2. It moves threading and all sleeping operations out of the decompressors. In doing so, it makes the decompressors non-blocking wrappers which only deal with interfacing with the decompressor implementation. 3. It splits decompressor.[ch] into decompressor generic functions in decompressor.[ch], and moves the single threaded decompressor implementation into decompressor_single.c. The result of this patch is Squashfs should now be able to support multiple decompressors by adding new decompressor_xxx.c files with specialised implementations of the functions in decompressor_single.c V3: * decompressor_single.c: Remove kfree(comp_opts) in error path * of squashfs_decompressor_create() * Double free of comp_opts found by static analysis, reported by * Dan Carpenter Signed-off-by: Phillip Lougher phil...@squashfs.org.uk Reviewed-by: Minchan Kim minc...@kernel.org -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] Squashfs: Refactor decompressor interface and code (V3)
The decompressor interface and code was written from
the point of view of single-threaded operation. In doing
so it mixed a lot of single-threaded implementation specific
aspects into the decompressor code and elsewhere which makes it
difficult to seamlessly support multiple different decompressor
implementations.
This patch does the following:
1. It removes compressor_options parsing from the decompressor
init() function. This allows the decompressor init() function
to be dynamically called to instantiate multiple decompressors,
without the compressor options needing to be read and parsed each
time.
2. It moves threading and all sleeping operations out of the
decompressors. In doing so, it makes the decompressors
non-blocking wrappers which only deal with interfacing with
the decompressor implementation.
3. It splits decompressor.[ch] into decompressor generic functions
in decompressor.[ch], and moves the single threaded
decompressor implementation into decompressor_single.c.
The result of this patch is Squashfs should now be able to
support multiple decompressors by adding new decompressor_xxx.c
files with specialised implementations of the functions in
decompressor_single.c
V3:
* decompressor_single.c: Remove kfree(comp_opts) in error path
* of squashfs_decompressor_create()
* Double free of comp_opts found by static analysis, reported by
* Dan Carpenter
Signed-off-by: Phillip Lougher
---
fs/squashfs/Makefile |2 +-
fs/squashfs/block.c | 11 +++--
fs/squashfs/decompressor.c| 47 +---
fs/squashfs/decompressor.h| 21 +++--
fs/squashfs/decompressor_single.c | 86 +++
fs/squashfs/lzo_wrapper.c | 24 +++---
fs/squashfs/squashfs.h|9 +++-
fs/squashfs/squashfs_fs_sb.h |3 +-
fs/squashfs/super.c | 10 ++---
fs/squashfs/xz_wrapper.c | 89 -
fs/squashfs/zlib_wrapper.c| 50 +++--
11 files changed, 216 insertions(+), 136 deletions(-)
create mode 100644 fs/squashfs/decompressor_single.c
diff --git a/fs/squashfs/Makefile b/fs/squashfs/Makefile
index 110b047..c223c84 100644
--- a/fs/squashfs/Makefile
+++ b/fs/squashfs/Makefile
@@ -4,7 +4,7 @@
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 decompressor.o
+squashfs-y += namei.o super.o symlink.o decompressor.o decompressor_single.o
squashfs-$(CONFIG_SQUASHFS_XATTR) += xattr.o xattr_id.o
squashfs-$(CONFIG_SQUASHFS_LZO) += lzo_wrapper.o
squashfs-$(CONFIG_SQUASHFS_XZ) += xz_wrapper.o
diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c
index 41d108e..4dd4025 100644
--- a/fs/squashfs/block.c
+++ b/fs/squashfs/block.c
@@ -93,7 +93,7 @@ int squashfs_read_data(struct super_block *sb, void **buffer,
u64 index,
struct buffer_head **bh;
int offset = index & ((1 << msblk->devblksize_log2) - 1);
u64 cur_index = index >> msblk->devblksize_log2;
- int bytes, compressed, b = 0, k = 0, page = 0, avail;
+ int bytes, compressed, b = 0, k = 0, page = 0, avail, i;
bh = kcalloc(((srclength + msblk->devblksize - 1)
>> msblk->devblksize_log2) + 1, sizeof(*bh), GFP_KERNEL);
@@ -158,6 +158,12 @@ int squashfs_read_data(struct super_block *sb, void
**buffer, u64 index,
ll_rw_block(READ, b - 1, bh + 1);
}
+ for (i = 0; i < b; i++) {
+ wait_on_buffer(bh[i]);
+ if (!buffer_uptodate(bh[i]))
+ goto block_release;
+ }
+
if (compressed) {
length = squashfs_decompress(msblk, buffer, bh, b, offset,
length, srclength, pages);
@@ -172,9 +178,6 @@ int squashfs_read_data(struct super_block *sb, void
**buffer, u64 index,
for (bytes = length; k < b; k++) {
in = min(bytes, msblk->devblksize - offset);
bytes -= in;
- wait_on_buffer(bh[k]);
- if (!buffer_uptodate(bh[k]))
- goto block_release;
while (in) {
if (pg_offset == PAGE_CACHE_SIZE) {
page++;
diff --git a/fs/squashfs/decompressor.c b/fs/squashfs/decompressor.c
index 3f6271d..234291f 100644
--- a/fs/squashfs/decompressor.c
+++ b/fs/squashfs/decompressor.c
@@ -37,29 +37,29 @@
*/
static const struct squashfs_decompressor squashfs_lzma_unsupported_comp_ops =
{
- NULL, NULL, NULL, LZMA_COMPRESSION, "lzma", 0
+ NULL, NULL, NULL, NULL, LZMA_COMPRESSION, "lzma", 0
};
#ifndef CONFIG_SQUASHFS_LZO
static const struct squashfs_decompressor squashfs_lzo_comp_ops = {
- NULL, NULL, NULL, LZO_COMPRESSION, "lzo", 0
+
[PATCH 1/4] Squashfs: Refactor decompressor interface and code (V3)
The decompressor interface and code was written from
the point of view of single-threaded operation. In doing
so it mixed a lot of single-threaded implementation specific
aspects into the decompressor code and elsewhere which makes it
difficult to seamlessly support multiple different decompressor
implementations.
This patch does the following:
1. It removes compressor_options parsing from the decompressor
init() function. This allows the decompressor init() function
to be dynamically called to instantiate multiple decompressors,
without the compressor options needing to be read and parsed each
time.
2. It moves threading and all sleeping operations out of the
decompressors. In doing so, it makes the decompressors
non-blocking wrappers which only deal with interfacing with
the decompressor implementation.
3. It splits decompressor.[ch] into decompressor generic functions
in decompressor.[ch], and moves the single threaded
decompressor implementation into decompressor_single.c.
The result of this patch is Squashfs should now be able to
support multiple decompressors by adding new decompressor_xxx.c
files with specialised implementations of the functions in
decompressor_single.c
V3:
* decompressor_single.c: Remove kfree(comp_opts) in error path
* of squashfs_decompressor_create()
* Double free of comp_opts found by static analysis, reported by
* Dan Carpenter
Signed-off-by: Phillip Lougher phil...@squashfs.org.uk
---
fs/squashfs/Makefile |2 +-
fs/squashfs/block.c | 11 +++--
fs/squashfs/decompressor.c| 47 +---
fs/squashfs/decompressor.h| 21 +++--
fs/squashfs/decompressor_single.c | 86 +++
fs/squashfs/lzo_wrapper.c | 24 +++---
fs/squashfs/squashfs.h|9 +++-
fs/squashfs/squashfs_fs_sb.h |3 +-
fs/squashfs/super.c | 10 ++---
fs/squashfs/xz_wrapper.c | 89 -
fs/squashfs/zlib_wrapper.c| 50 +++--
11 files changed, 216 insertions(+), 136 deletions(-)
create mode 100644 fs/squashfs/decompressor_single.c
diff --git a/fs/squashfs/Makefile b/fs/squashfs/Makefile
index 110b047..c223c84 100644
--- a/fs/squashfs/Makefile
+++ b/fs/squashfs/Makefile
@@ -4,7 +4,7 @@
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 decompressor.o
+squashfs-y += namei.o super.o symlink.o decompressor.o decompressor_single.o
squashfs-$(CONFIG_SQUASHFS_XATTR) += xattr.o xattr_id.o
squashfs-$(CONFIG_SQUASHFS_LZO) += lzo_wrapper.o
squashfs-$(CONFIG_SQUASHFS_XZ) += xz_wrapper.o
diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c
index 41d108e..4dd4025 100644
--- a/fs/squashfs/block.c
+++ b/fs/squashfs/block.c
@@ -93,7 +93,7 @@ int squashfs_read_data(struct super_block *sb, void **buffer,
u64 index,
struct buffer_head **bh;
int offset = index ((1 msblk-devblksize_log2) - 1);
u64 cur_index = index msblk-devblksize_log2;
- int bytes, compressed, b = 0, k = 0, page = 0, avail;
+ int bytes, compressed, b = 0, k = 0, page = 0, avail, i;
bh = kcalloc(((srclength + msblk-devblksize - 1)
msblk-devblksize_log2) + 1, sizeof(*bh), GFP_KERNEL);
@@ -158,6 +158,12 @@ int squashfs_read_data(struct super_block *sb, void
**buffer, u64 index,
ll_rw_block(READ, b - 1, bh + 1);
}
+ for (i = 0; i b; i++) {
+ wait_on_buffer(bh[i]);
+ if (!buffer_uptodate(bh[i]))
+ goto block_release;
+ }
+
if (compressed) {
length = squashfs_decompress(msblk, buffer, bh, b, offset,
length, srclength, pages);
@@ -172,9 +178,6 @@ int squashfs_read_data(struct super_block *sb, void
**buffer, u64 index,
for (bytes = length; k b; k++) {
in = min(bytes, msblk-devblksize - offset);
bytes -= in;
- wait_on_buffer(bh[k]);
- if (!buffer_uptodate(bh[k]))
- goto block_release;
while (in) {
if (pg_offset == PAGE_CACHE_SIZE) {
page++;
diff --git a/fs/squashfs/decompressor.c b/fs/squashfs/decompressor.c
index 3f6271d..234291f 100644
--- a/fs/squashfs/decompressor.c
+++ b/fs/squashfs/decompressor.c
@@ -37,29 +37,29 @@
*/
static const struct squashfs_decompressor squashfs_lzma_unsupported_comp_ops =
{
- NULL, NULL, NULL, LZMA_COMPRESSION, lzma, 0
+ NULL, NULL, NULL, NULL, LZMA_COMPRESSION, lzma, 0
};
#ifndef CONFIG_SQUASHFS_LZO
static const struct squashfs_decompressor squashfs_lzo_comp_ops = {
- NULL, NULL, NULL, LZO_COMPRESSION, lzo,
