From: Ming Lei <ming.lei@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>, linux-block@vger.kernel.org
Subject: Re: [PATCH 8/8] block: never take page references for ITER_BVEC
Date: Mon, 6 May 2019 16:19:54 +0800 [thread overview]
Message-ID: <20190506081952.GA24702@ming.t460p> (raw)
In-Reply-To: <20190502233332.28720-9-hch@lst.de>
On Thu, May 02, 2019 at 07:33:32PM -0400, Christoph Hellwig wrote:
> If we pass pages through an iov_iter we always already have a reference
> in the caller. Thus remove the ITER_BVEC_FLAG_NO_REF and don't take
> reference to pages by default for bvec backed iov_iters.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> block/bio.c | 14 +-------------
> drivers/block/loop.c | 16 ++++------------
> fs/io_uring.c | 3 ---
> include/linux/uio.h | 8 --------
> 4 files changed, 5 insertions(+), 36 deletions(-)
>
> diff --git a/block/bio.c b/block/bio.c
> index 3938e179a530..e999d530d863 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -846,15 +846,6 @@ int bio_add_page(struct bio *bio, struct page *page,
> }
> EXPORT_SYMBOL(bio_add_page);
>
> -static void bio_get_pages(struct bio *bio)
> -{
> - struct bvec_iter_all iter_all;
> - struct bio_vec *bvec;
> -
> - bio_for_each_segment_all(bvec, bio, iter_all)
> - get_page(bvec->bv_page);
> -}
> -
> void bio_release_pages(struct bio *bio)
> {
> struct bvec_iter_all iter_all;
> @@ -967,11 +958,8 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
> ret = __bio_iov_iter_get_pages(bio, iter);
> } while (!ret && iov_iter_count(iter) && !bio_full(bio));
>
> - if (iov_iter_bvec_no_ref(iter))
> + if (is_bvec)
> bio_set_flag(bio, BIO_NO_PAGE_REF);
> - else if (is_bvec)
> - bio_get_pages(bio);
> -
> return bio->bi_vcnt ? 0 : ret;
> }
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 102d79575895..c20710e617c2 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -264,20 +264,12 @@ lo_do_transfer(struct loop_device *lo, int cmd,
> return ret;
> }
>
> -static inline void loop_iov_iter_bvec(struct iov_iter *i,
> - unsigned int direction, const struct bio_vec *bvec,
> - unsigned long nr_segs, size_t count)
> -{
> - iov_iter_bvec(i, direction, bvec, nr_segs, count);
> - i->type |= ITER_BVEC_FLAG_NO_REF;
> -}
> -
> static int lo_write_bvec(struct file *file, struct bio_vec *bvec, loff_t *ppos)
> {
> struct iov_iter i;
> ssize_t bw;
>
> - loop_iov_iter_bvec(&i, WRITE, bvec, 1, bvec->bv_len);
> + iov_iter_bvec(&i, WRITE, bvec, 1, bvec->bv_len);
>
> file_start_write(file);
> bw = vfs_iter_write(file, &i, ppos, 0);
> @@ -355,7 +347,7 @@ static int lo_read_simple(struct loop_device *lo, struct request *rq,
> ssize_t len;
>
> rq_for_each_segment(bvec, rq, iter) {
> - loop_iov_iter_bvec(&i, READ, &bvec, 1, bvec.bv_len);
> + iov_iter_bvec(&i, READ, &bvec, 1, bvec.bv_len);
> len = vfs_iter_read(lo->lo_backing_file, &i, &pos, 0);
> if (len < 0)
> return len;
> @@ -396,7 +388,7 @@ static int lo_read_transfer(struct loop_device *lo, struct request *rq,
> b.bv_offset = 0;
> b.bv_len = bvec.bv_len;
>
> - loop_iov_iter_bvec(&i, READ, &b, 1, b.bv_len);
> + iov_iter_bvec(&i, READ, &b, 1, b.bv_len);
> len = vfs_iter_read(lo->lo_backing_file, &i, &pos, 0);
> if (len < 0) {
> ret = len;
> @@ -563,7 +555,7 @@ static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd,
> }
> atomic_set(&cmd->ref, 2);
>
> - loop_iov_iter_bvec(&iter, rw, bvec, nr_bvec, blk_rq_bytes(rq));
> + iov_iter_bvec(&iter, rw, bvec, nr_bvec, blk_rq_bytes(rq));
> iter.iov_offset = offset;
>
> cmd->iocb.ki_pos = pos;
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index f65f85d89217..f7eb63a5b3db 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -853,9 +853,6 @@ static int io_import_fixed(struct io_ring_ctx *ctx, int rw,
> iov_iter_bvec(iter, rw, imu->bvec, imu->nr_bvecs, offset + len);
> if (offset)
> iov_iter_advance(iter, offset);
> -
> - /* don't drop a reference to these pages */
> - iter->type |= ITER_BVEC_FLAG_NO_REF;
> return 0;
> }
>
> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index f184af1999a8..bace8fd40d0c 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -23,9 +23,6 @@ struct kvec {
> };
>
> enum iter_type {
> - /* set if ITER_BVEC doesn't hold a bv_page ref */
> - ITER_BVEC_FLAG_NO_REF = 2,
> -
> /* iter types */
> ITER_IOVEC = 4,
> ITER_KVEC = 8,
> @@ -93,11 +90,6 @@ static inline unsigned char iov_iter_rw(const struct iov_iter *i)
> return i->type & (READ | WRITE);
> }
>
> -static inline bool iov_iter_bvec_no_ref(const struct iov_iter *i)
> -{
> - return (i->type & ITER_BVEC_FLAG_NO_REF) != 0;
> -}
> -
> /*
> * Total number of bytes covered by an iovec.
> *
I remember that this way is the initial version of Jens' patch, however
kernel bug is triggered:
https://lore.kernel.org/linux-block/20190226034613.GA676@sol.localdomain/
Or maybe I miss some recent changes, could you explain it a bit?
Thanks,
Ming
next prev parent reply other threads:[~2019-05-06 8:22 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-02 23:33 cleanup bio page releasing and fix a page leak Christoph Hellwig
2019-05-02 23:33 ` [PATCH 1/8] block: move the BIO_NO_PAGE_REF check into bio_release_pages Christoph Hellwig
2019-05-04 12:28 ` Johannes Thumshirn
2019-05-02 23:33 ` [PATCH 2/8] block: use bio_release_pages in bio_unmap_user Christoph Hellwig
2019-05-03 6:50 ` Nikolay Borisov
2019-05-03 7:20 ` Christoph Hellwig
2019-05-02 23:33 ` [PATCH 3/8] block: use bio_release_pages in bio_map_user_iov Christoph Hellwig
2019-05-04 12:31 ` Johannes Thumshirn
2019-05-02 23:33 ` [PATCH 4/8] iomap: use bio_release_pages in iomap_dio_bio_end_io Christoph Hellwig
2019-05-04 12:33 ` Johannes Thumshirn
2019-05-04 12:35 ` Johannes Thumshirn
2019-05-02 23:33 ` [PATCH 5/8] block_dev: use bio_release_pages in blkdev_bio_end_io Christoph Hellwig
2019-05-04 12:35 ` Johannes Thumshirn
2019-05-02 23:33 ` [PATCH 6/8] block_dev: use bio_release_pages in __blkdev_direct_IO_simple Christoph Hellwig
2019-05-04 17:38 ` Johannes Thumshirn
2019-05-02 23:33 ` [PATCH 7/8] direct-io: use bio_release_pages in dio_bio_complete Christoph Hellwig
2019-05-04 17:39 ` Johannes Thumshirn
2019-05-02 23:33 ` [PATCH 8/8] block: never take page references for ITER_BVEC Christoph Hellwig
2019-05-04 17:41 ` Johannes Thumshirn
2019-05-06 8:19 ` Ming Lei [this message]
2019-05-06 13:30 ` Christoph Hellwig
2019-05-07 6:09 ` Christoph Hellwig
2019-05-03 17:52 ` cleanup bio page releasing and fix a page leak Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190506081952.GA24702@ming.t460p \
--to=ming.lei@redhat.com \
--cc=axboe@kernel.dk \
--cc=hch@lst.de \
--cc=linux-block@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).