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

[Phillip Lougher <phillip.lougher@gmail.com>]

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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>