| Message ID | 20230123173007.325544-5-dhowells@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iov_iter: Improve page extraction (pin or just list) | expand |
On Mon, Jan 23, 2023 at 05:30:01PM +0000, David Howells wrote: > From: Christoph Hellwig <hch@lst.de> > > ZERO_PAGE can't go away, no need to hold an extra reference. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: David Howells <dhowells@redhat.com> I did originally attribute this to you as it's just extracted as an intermedia step from your patch. But I can live with it being credited to me if you want.
On 1/23/23 09:30, David Howells wrote: > From: Christoph Hellwig <hch@lst.de> > > ZERO_PAGE can't go away, no need to hold an extra reference. That statement is true, but... > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: David Howells <dhowells@redhat.com> > --- > fs/iomap/direct-io.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c > index 9804714b1751..47db4ead1e74 100644 > --- a/fs/iomap/direct-io.c > +++ b/fs/iomap/direct-io.c > @@ -202,7 +202,7 @@ static void iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio, > bio->bi_private = dio; > bio->bi_end_io = iomap_dio_bio_end_io; > > - get_page(page); > + bio_set_flag(bio, BIO_NO_PAGE_REF); ...is it accurate to assume that the entire bio is pointing to the zero page? I recall working through this area earlier last year, and ended up just letting the zero page get pinned, and then unpinning upon release, which is harmless. I like your approach, if it is possible. I'm just not sure that it's correct given that bio's can have more than one page. thanks,
On Mon, Jan 23, 2023 at 06:42:28PM -0800, John Hubbard wrote: > > @@ -202,7 +202,7 @@ static void iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio, > > bio->bi_private = dio; > > bio->bi_end_io = iomap_dio_bio_end_io; > > - get_page(page); > > + bio_set_flag(bio, BIO_NO_PAGE_REF); > > ...is it accurate to assume that the entire bio is pointing to the zero > page? I recall working through this area earlier last year, and ended up > just letting the zero page get pinned, and then unpinning upon release, > which is harmless. Yes, the bio is built 4 lines above what is quoted here, and submitted right after it. It only contains the ZERO_PAGE.
On 1/23/23 21:59, Christoph Hellwig wrote: > On Mon, Jan 23, 2023 at 06:42:28PM -0800, John Hubbard wrote: >>> @@ -202,7 +202,7 @@ static void iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio, >>> bio->bi_private = dio; >>> bio->bi_end_io = iomap_dio_bio_end_io; >>> - get_page(page); >>> + bio_set_flag(bio, BIO_NO_PAGE_REF); >> >> ...is it accurate to assume that the entire bio is pointing to the zero >> page? I recall working through this area earlier last year, and ended up >> just letting the zero page get pinned, and then unpinning upon release, >> which is harmless. > > Yes, the bio is built 4 lines above what is quoted here, and submitted > right after it. It only contains the ZERO_PAGE. OK, yes. All good, then. thanks,
On 23.01.23 18:30, David Howells wrote: > From: Christoph Hellwig <hch@lst.de> > > ZERO_PAGE can't go away, no need to hold an extra reference. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: David Howells <dhowells@redhat.com> > --- > fs/iomap/direct-io.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c > index 9804714b1751..47db4ead1e74 100644 > --- a/fs/iomap/direct-io.c > +++ b/fs/iomap/direct-io.c > @@ -202,7 +202,7 @@ static void iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio, > bio->bi_private = dio; > bio->bi_end_io = iomap_dio_bio_end_io; > > - get_page(page); > + bio_set_flag(bio, BIO_NO_PAGE_REF); > __bio_add_page(bio, page, len, 0); > iomap_dio_submit_bio(iter, dio, bio, pos); > } > Reviewed-by: David Hildenbrand <david@redhat.com>
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c index 9804714b1751..47db4ead1e74 100644 --- a/fs/iomap/direct-io.c +++ b/fs/iomap/direct-io.c @@ -202,7 +202,7 @@ static void iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio, bio->bi_private = dio; bio->bi_end_io = iomap_dio_bio_end_io; - get_page(page); + bio_set_flag(bio, BIO_NO_PAGE_REF); __bio_add_page(bio, page, len, 0); iomap_dio_submit_bio(iter, dio, bio, pos); }