Re: dm: don't reuse bio for flushes

[Jens Axboe <axboe@kernel.dk>]

On 12/19/18 9:13 AM, Mike Snitzer wrote:
> On Wed, Dec 19 2018 at 11:11am -0500,
> Mike Snitzer <snitzer@redhat.com> wrote:
> 
>> On Wed, Dec 19 2018 at 10:50am -0500,
>> Jens Axboe <axboe@kernel.dk> wrote:
>>
>>> DM currently has a statically allocated bio that it uses to issue empty
>>> flushes. It doesn't submit this bio, it just uses it for maintaining
>>> state while setting up clones. Multiple users can access this bio at the
>>> same time. This wasn't previously an issue, even if it was a bit iffy,
>>> but with the blkg associations it can become one.
>>>
>>> We setup the blkg association, then clone bio's and submit, then remove
>>> the blkg assocation again. But since we can have multiple tasks doing
>>> this at the same time, against multiple blkg's, then we can either lose
>>> references to a blkg, or put it twice. The latter causes complaints on
>>> the percpu ref being <= 0 when released, and can cause use-after-free as
>>> well. Ming reports that xfstest generic/475 triggers this:
>>>
>>> ------------[ cut here ]------------
>>> percpu ref (blkg_release) <= 0 (0) after switching to atomic
>>> WARNING: CPU: 13 PID: 0 at lib/percpu-refcount.c:155 percpu_ref_switch_to_atomic_rcu+0x2c9/0x4a0
>>>
>>> Switch to just using an on-stack bio for this, and get rid of the
>>> embedded bio.
>>>
>>> Fixes: 5cdf2e3fea5e ("blkcg: associate blkg when associating a device")
>>> Reported-by: Ming Lei <ming.lei@redhat.com>
>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>
>> Thanks for sorting this one out, definitely wasn't happy with how
>> exposed DM was left with the recent blkg changes.  This is clearly
>> better.
>>
>> Acked-by: Mike Snitzer <snitzer@redhat.com>
> 
> Please upgrade this to:
> 
> Reviewed-by: Mike Snitzer <snitzer@redhat.com>

Done, thanks Mike.

-- 
Jens Axboe