On Fri 22-11-19 05:26:58, Christoph Hellwig wrote:
> > - /*
> > - * Operate on a partial iter trimmed to the extent we were called for.
> > - * We'll update the iter in the dio once we're done with this extent.
> > - */
> > - iter = *dio->submit.iter;
> > - iov_iter_truncate(&iter, length);
> > + /* Operate on a partial iter trimmed to the extent we were called for */
> > + iov_iter_truncate(dio->submit.iter, length);
>
> I think the comment could be kept a little more verbose given that the
> scheme isn't exactly obvious. Also I'd move the initialization of
> orig_count here to keep it all together. E.g.
>
> /*
> * Save the original count and trim the iter to just the extent we
> * are operating on right now. The iter will be re-expanded once
> * we are done.
> */
> orig_count = iov_iter_count(dio->submit.iter);
> iov_iter_truncate(dio->submit.iter, length);
>
> >
> > - nr_pages = iov_iter_npages(&iter, BIO_MAX_PAGES);
> > - if (nr_pages <= 0)
> > + nr_pages = iov_iter_npages(dio->submit.iter, BIO_MAX_PAGES);
> > + if (nr_pages <= 0) {
> > + iov_iter_reexpand(dio->submit.iter, orig_count);
> > return nr_pages;
> > + }
>
> Can we stick to a single iov_iter_reexpand call? E.g. turn this into
>
> if (nr_pages <= 0) {
> ret = nr_pages;
> goto out;
> }
>
> and then have the out label at the very end call iov_iter_reexpand.
>
> > iomap_dio_zero(dio, iomap, pos, fs_block_size - pad);
> > }
> > + /* Undo iter limitation to current extent */
> > + iov_iter_reexpand(dio->submit.iter, orig_count - copied);
> > return copied ? copied : ret;
>
> In iomap-for-next this is:
>
> if (copied)
> return copied;
> return ret;
>
> so please rebase to iomap-for-next for the next spin.
OK, I can see Darrick has already picked up the first patch so I'll just
respin this second one with the updates you've asked for. Thanks for
review!
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
