| Message ID | 20201202062227.9826-2-chaitanya.kulkarni@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nvmet: add ZBD backend support | expand |
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
On Tue, Dec 01, 2020 at 10:22:19PM -0800, Chaitanya Kulkarni wrote: > Remove the bvec check in the bio_iov_iter_get_pages() for > REQ_OP_ZONE_APPEND so that we can reuse the code and build iter from > bvec. We should do the same optimization for bvec pages that the normal path does. That being said using bio_iov_iter_get_pages in nvmet does not make any sense to me whatsover.
On 12/2/20 00:55, Christoph Hellwig wrote: > On Tue, Dec 01, 2020 at 10:22:19PM -0800, Chaitanya Kulkarni wrote: >> Remove the bvec check in the bio_iov_iter_get_pages() for >> REQ_OP_ZONE_APPEND so that we can reuse the code and build iter from >> bvec. > We should do the same optimization for bvec pages that the normal path > does. That being said using bio_iov_iter_get_pages in nvmet does not > make any sense to me whatsover. > Are you referring to the inline bvec ? then yes, I'll add it in next version. I did not understand bio_iov_iter_get_pages() comment though. Reimplementing the bio loop over sg with the use of bio_add_hw_page() seems repetition of the code which we already have in bio_iov_iter_get_pages(). Can you please explain why bio_iov_iter_get_pages() not the right way ?
On Fri, Dec 04, 2020 at 02:43:10AM +0000, Chaitanya Kulkarni wrote: > >> Remove the bvec check in the bio_iov_iter_get_pages() for > >> REQ_OP_ZONE_APPEND so that we can reuse the code and build iter from > >> bvec. > > We should do the same optimization for bvec pages that the normal path > > does. That being said using bio_iov_iter_get_pages in nvmet does not > > make any sense to me whatsover. > > > Are you referring to the inline bvec ? then yes, I'll add it in next > version. > > I did not understand bio_iov_iter_get_pages() comment though. > > Reimplementing the bio loop over sg with the use of bio_add_hw_page() seems > > repetition of the code which we already have in bio_iov_iter_get_pages(). > > > Can you please explain why bio_iov_iter_get_pages() not the right way ? bio_iov_iter_get_pages is highly inefficient for this use case, as we'll need to allocate two sets of bio_vecs. It also is rather redundant as Zone Append should be able to just largely reuse the write path.
diff --git a/block/bio.c b/block/bio.c index fa01bef35bb1..54b532bb39f3 100644 --- a/block/bio.c +++ b/block/bio.c @@ -1110,8 +1110,6 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter) do { if (bio_op(bio) == REQ_OP_ZONE_APPEND) { - if (WARN_ON_ONCE(is_bvec)) - return -EINVAL; ret = __bio_iov_append_get_pages(bio, iter); } else { if (is_bvec)
Remove the bvec check in the bio_iov_iter_get_pages() for REQ_OP_ZONE_APPEND so that we can reuse the code and build iter from bvec. Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> --- block/bio.c | 2 -- 1 file changed, 2 deletions(-)