On Fri, 2018-07-20 at 17:11 +0200, Christoph Hellwig wrote:
> 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.
If that's consensus, I'll do it happily. So far I felt there was
opposition against it.
> > + 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.
OK, will do.
I'll wait for some more feedback on the "fix bio_iov_iter_get_pages"
part.
Martin
--
Dr. Martin Wilck <mwilck@xxxxxxxx>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
