Re: [PATCH v4 3/4] block: add bio_iov_iter_get_all_pages() helper

[Christoph Hellwig <hch@lst.de>]

On Fri, Jul 20, 2018 at 03:05:51PM +0200, Martin Wilck wrote:
> bio_iov_iter_get_pages() only adds pages for the next non-zero
> segment from the iov_iter to the bio. Some callers prefer to
> obtain as many pages as would fit into the bio, with proper
> rollback in case of failure. Add bio_iov_iter_get_all_pages()
> for this purpose.

I'd much rather have you fix bio_iov_iter_get_pages.  It only has
three callers, all beeing slight variations of the same direct I/O
pattern.  There is no point in diverging in implementation details
for them.

> +	do {
> +		int ret = bio_iov_iter_get_pages(bio, iter);
> +
> +		if (unlikely(ret)) {
> +			struct bio_vec *bvec;
> +			unsigned short i;
> +
> +			bio_for_each_segment_all(bvec, bio, i) {
> +				if (i >= orig_vcnt) {
> +					put_page(bvec->bv_page);
> +					bvec->bv_page = NULL;
> +					bvec->bv_len = 0;
> +					bvec->bv_offset = 0;
> +				}
> +			}

I don't think we need any of the zeroing here.   Also for code flow
purposes I'd rather see a goto for the error handling here.