Re: [PATCHv5 4/9] blk-integrity: consider entire bio list for merging

Christoph Hellwig
2024-09-16T06:44:22Z

On Sat, Sep 14, 2024 at 10:51:10AM -0600, Keith Busch wrote:
> On Sat, Sep 14, 2024 at 09:30:12AM +0200, Christoph Hellwig wrote:
> > On Fri, Sep 13, 2024 at 11:28:49AM -0700, Keith Busch wrote:
> > > From: Keith Busch <kbusch@kernel.org>
> > >
> > > If a bio is merged to a request, the entire bio list is merged, so don't
> > > temporarily detach it from its list when counting segments. In most
> > > cases, bi_next will already be NULL, so detaching is usually a no-op.
> > > But if the bio does have a list, the current code is miscounting the
> > > segments for the resulting merge.
> >
> > As I explained in detail last round this is still wrong. There is
> > no bio list here ever.
>
> Could you explain "wrong"? If we assume there is never bio list here,
> then the current code is performing a useless no-op, and this patch
> removes it. That's a good thing, no?

The wrong thing primarily is the above commit log. The code change
itself is correct, but we'd be better off killing the iteration over
the bio chain as well to make the code less confusing.