[dm-devel] linux-next dm-thinp regression due to "bio: skip atomic inc/dec of ->bi_remaining for non-chains"

Mike Snitzer snitzer at redhat.com
Thu May 14 03:29:53 UTC 2015


On Wed, May 13 2015 at 10:49pm -0400,
Mike Snitzer <snitzer at redhat.com> wrote:
 
> As you know commit c4cf5261 avoids the atomic operations during endio
> unless BIO_CHAIN is set.  It only gets set if bio_inc_remaining() is
> used.  But bio_inc_remaining() is only used _after_ the initial endio --
> we needed BIO_CHAIN set earlier for these cases.  Seems we need a new
> function to register the intent to cascade endios (to allow for proper
> accounting)?
> 
> We could then switch callers to something like:
> 
>   bio_set_chain(bio); // establishes BIO_CHAIN iff __bio_remaining is 1?
>   old = io->bi_end_io;
>   bio->bi_end_io = new;
> 
>  in new():
>   bio->bi_end_io = old;
>   bio_inc_remaining(bio)
> 
> Anyway, not sure about the proper fix -- I do know this commit is a
> definite regression (but it does nicely avoid the costly atomics ;)

Just adding this to dm-thin.c:save_and_set_endio() fixed it:
 bio->bi_flags |= (1 << BIO_CHAIN);

(setting a bit doesn't need a bio_set_chain fn like I suggested above).

But, all relevant callers would need this treatment (I'd imagine
bio_endio_nodec callers need help too? -- either that or __bi_remaining
is completely irrelevant for all old users except for bio_chain()).




More information about the dm-devel mailing list