From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from verein.lst.de ([213.95.11.211]:58470 "EHLO newverein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732738AbeLULcl (ORCPT ); Fri, 21 Dec 2018 06:32:41 -0500 Date: Fri, 21 Dec 2018 12:32:39 +0100 From: Christoph Hellwig To: Jens Axboe Cc: linux-fsdevel@vger.kernel.org, linux-aio@kvack.org, hch@lst.de, viro@zeniv.linux.org.uk Subject: Re: [PATCH 17/22] block: implement bio helper to add iter bvec pages to bio Message-ID: <20181221113239.GF7319@lst.de> References: <20181218154230.3120-1-axboe@kernel.dk> <20181218154230.3120-18-axboe@kernel.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181218154230.3120-18-axboe@kernel.dk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: I don't think we need the loop for the ITER_BVEC case as bio_Add_page will only fail when the bio is full. Something like this would simplify the implementation: diff --git a/block/bio.c b/block/bio.c index cc1ddf173aaf..658dcfc8d216 100644 --- a/block/bio.c +++ b/block/bio.c @@ -831,18 +831,18 @@ EXPORT_SYMBOL(bio_add_page); static int __bio_iov_bvec_add_pages(struct bio *bio, struct iov_iter *iter) { const struct bio_vec *bv = iter->bvec; - unsigned int len; - size_t size; - - len = min_t(size_t, bv->bv_len, iter->count); - size = bio_add_page(bio, bv->bv_page, len, - bv->bv_offset + iter->iov_offset); - if (size == len) { - iov_iter_advance(iter, size); - return 0; - } + unsigned int len = min_t(size_t, bv->bv_len, iter->count); + unsigned int offset = bv->bv_offset + iter->iov_offset; - return -EINVAL; + /* + * If this is a BVEC iter, then the pages are kernel pages. Don't + * release them on IO completion. + */ + bio_set_flag(bio, BIO_HOLD_PAGES); + if (bio_add_page(bio, bv->bv_page, len, offset) != len) + return -EINVAL; + iov_iter_advance(iter, len); + return 0; } #define PAGE_PTRS_PER_BVEC (sizeof(struct bio_vec) / sizeof(struct page *)) @@ -913,24 +913,13 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter) */ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter) { - const bool is_bvec = iov_iter_is_bvec(iter); unsigned short orig_vcnt = bio->bi_vcnt; - /* - * If this is a BVEC iter, then the pages are kernel pages. Don't - * release them on IO completion. - */ - if (is_bvec) - bio_set_flag(bio, BIO_HOLD_PAGES); + if (iov_iter_is_bvec(iter)) + return __bio_iov_bvec_add_pages(bio, iter); do { - int ret; - - if (is_bvec) - ret = __bio_iov_bvec_add_pages(bio, iter); - else - ret = __bio_iov_iter_get_pages(bio, iter); - + int ret = __bio_iov_iter_get_pages(bio, iter); if (unlikely(ret)) return bio->bi_vcnt > orig_vcnt ? 0 : ret;