Re: [PATCH 6/7] Squashfs: Directly decompress into the page cachefor file data

From: Phillip Lougher
Date: Sat Nov 23 2013 - 20:19:41 EST


On 22/11/13 07:40, Roman Peniaev wrote:
Hello, Phillip.

one remark below:


+static int squashfs_read_cache(struct page *target_page, u64 block, int
bsize,
+ int pages, struct page **page)
+{
+ struct inode *i = target_page->mapping->host;
+ struct squashfs_cache_entry *buffer = squashfs_get_datablock(i-
i_sb,
+ block, bsize);
+ int bytes = buffer->length, res = buffer->error, n, offset = 0;
+ void *pageaddr;
+
+ if (res) {
+ ERROR("Unable to read page, block %llx, size %x\n", block,
+ bsize);
+ goto out;


have you forgotten to unlock the pages on error path?

in case of error squashfs_readpage will unlock only target page.

Yup, a fix for -rc1 is on its way.

This error path failed to trigger in my stress tests because it is
pretty hard to hit, because it relies on decompress failure at the
same time as we've raced against another process also grabbing
the pages.

Phillip




+ }
+
+ for (n = 0; n < pages && bytes > 0; n++,
+ bytes -= PAGE_CACHE_SIZE, offset +=
PAGE_CACHE_SIZE) {
+ int avail = min_t(int, bytes, PAGE_CACHE_SIZE);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/