Re: [PATCH] hfsplus: fix cross-page bio requests

[Viacheslav Dubeyko <slava@dubeyko.com>]

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;
>  }