From mboxrd@z Thu Jan  1 00:00:00 1970
From: Viacheslav Dubeyko <slava@dubeyko.com>
Subject: Re: [PATCH] hfsplus: fix cross-page bio requests
Date: Fri, 10 Apr 2015 09:48:20 -0700
Message-ID: <1428684500.2798.4.camel@slavad-ubuntu-14.04>
References: <1428656543-6790-1-git-send-email-saproj@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Cc: linux-fsdevel@vger.kernel.org,
	Anton Altaparmakov <aia21@cam.ac.uk>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Hin-Tak Leung <htl10@users.sourceforge.net>,
	Sougata Santra <sougata@tuxera.com>
To: Sergei Antonov <saproj@gmail.com>
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mail-pa0-f41.google.com ([209.85.220.41]:36699 "EHLO
	mail-pa0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1754739AbbDJQsX (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Fri, 10 Apr 2015 12:48:23 -0400
Received: by pabsx10 with SMTP id sx10so26658151pab.3
        for <linux-fsdevel@vger.kernel.org>; Fri, 10 Apr 2015 09:48:22 -0700 (PDT)
In-Reply-To: <1428656543-6790-1-git-send-email-saproj@gmail.com>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Fri, 2015-04-10 at 11:02 +0200, Sergei Antonov wrote:
> Function hfsplus_submit_bio() did not work when the passed buffer spanned
> over more than one page. That was because bio_alloc() is passed 1 as a number
> of vectors but more than one vector were added inside the 'while' loop.
> It periodically caused a mount error when the volume header could not be read.
> 
> This patch modifies the code so that only one vector is used. It works for
> multiple pages too. Also adds a return code check after bio_alloc().

I think that it really makes sense to describe the issue's reproducing
way. It will be really precious for understanding of symptoms and
reasons of the issue.

Could you add more detailed description?

Then, I will have opportunity to test your patch.

Thanks,
Vyacheslav Dubeyko.

> 
> Cc: Anton Altaparmakov <aia21@cam.ac.uk>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Vyacheslav Dubeyko <slava@dubeyko.com>
> Cc: Hin-Tak Leung <htl10@users.sourceforge.net>
> Cc: Sougata Santra <sougata@tuxera.com>
> Signed-off-by: Sergei Antonov <saproj@gmail.com>
> ---
>  fs/hfsplus/wrapper.c | 29 ++++++++++-------------------
>  1 file changed, 10 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/hfsplus/wrapper.c b/fs/hfsplus/wrapper.c
> index cc62356..e245faa 100644
> --- a/fs/hfsplus/wrapper.c
> +++ b/fs/hfsplus/wrapper.c
> @@ -62,29 +62,20 @@ int hfsplus_submit_bio(struct super_block *sb, sector_t sector,
>  	offset = start & (io_size - 1);
>  	sector &= ~((io_size >> HFSPLUS_SECTOR_SHIFT) - 1);
>  
> -	bio = bio_alloc(GFP_NOIO, 1);
> -	bio->bi_iter.bi_sector = sector;
> -	bio->bi_bdev = sb->s_bdev;
> -
>  	if (!(rw & WRITE) && data)
>  		*data = (u8 *)buf + offset;
>  
> -	while (io_size > 0) {
> -		unsigned int page_offset = offset_in_page(buf);
> -		unsigned int len = min_t(unsigned int, PAGE_SIZE - page_offset,
> -					 io_size);
> -
> -		ret = bio_add_page(bio, virt_to_page(buf), len, page_offset);
> -		if (ret != len) {
> -			ret = -EIO;
> -			goto out;
> -		}
> -		io_size -= len;
> -		buf = (u8 *)buf + len;
> -	}
> -
> +	bio = bio_alloc(GFP_NOIO, 1);
> +	if (!bio)
> +		return -ENOMEM;
> +	bio->bi_iter.bi_sector = sector;
> +	bio->bi_bdev = sb->s_bdev;
> +	bio->bi_vcnt = 1;
> +	bio->bi_iter.bi_size = io_size;
> +	bio->bi_io_vec[0].bv_page = virt_to_page(buf);
> +	bio->bi_io_vec[0].bv_offset = offset_in_page(buf);
> +	bio->bi_io_vec[0].bv_len = io_size;
>  	ret = submit_bio_wait(rw, bio);
> -out:
>  	bio_put(bio);
>  	return ret < 0 ? ret : 0;
>  }