On Tue, Jun 28, 2022 at 03:49:28PM +0800, Ming Lei wrote: > On Tue, Jun 28, 2022 at 12:26:10AM -0400, Kent Overstreet wrote: > > On Mon, Jun 27, 2022 at 03:36:22PM +0800, Ming Lei wrote: > > > Not mention bio_iter, bvec_iter has been 32 bytes, which is too big to > > > hold in per-io data structure. With this patch, 8bytes is enough > > > to rewind one bio if the end sector is fixed. > > > > And with rewind, you're making an assumption about the state the iterator is > > going to be in when the IO has completed. > > > > What if the iterator was never advanced? > > bio_rewind() works as expected if the iterator doesn't advance, since bytes > between the recorded position and the end position isn't changed, same > with the end position. > > > > > So say you check for that by saving some other part of the iterator - but that > > may have legitimately changed too, if the bio was redirected (bi_sector changes) > > or trimmed (bi_size changes) > > > > I still think this is an inherently buggy interface, the way it's being proposed > > to be used. > > The patch did mention that the interface should be for situation in which end > sector of bio won't change. But that's an assumption that you simply can't make! We allow block device drivers to be stacked in _any_ combination. After a bio is completed it may have been partially advanced, fully advanced, trimmed, not trimmed, anything - and bi_sector and thus also bio_end_sector() may have changed, and will have if there's partition tables involved.