[PATCH 0/3] offload bios to a thread

[PATCH 0/3] offload bios to a thread

[Mikulas Patocka]

Hi

Here I'm sending three patches to fix the deadlocks in snapshot and 
snapshot-merge.

The first patch fixes the deadlock, the following 2 patches introduce a 
timer, so that bios are not offloaded immediatelly, they are offloaded 
after a specified timeout, because immediate offloading can change order 
of bios and it could theoretically produce regressions. I don't know if 
these regressions really exist or not.

If there is some way to push the patches upstream, try it.

Mikulas

Re: [PATCH 0/3] offload bios to a thread

[Mike Snitzer]

[cc'ing linux-block and drbd folks]

On Tue, Jun 28 2016 at  8:16pm -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> Hi
> 
> Here I'm sending three patches to fix the deadlocks in snapshot and 
> snapshot-merge.
> 
> The first patch fixes the deadlock, the following 2 patches introduce a 
> timer, so that bios are not offloaded immediatelly, they are offloaded 
> after a specified timeout, because immediate offloading can change order 
> of bios and it could theoretically produce regressions. I don't know if 
> these regressions really exist or not.
> 
> If there is some way to push the patches upstream, try it.

Some fix must happen before the more recent upstream kernels can be
reliably used in stacked bio-based workloads (in production).  We simply
cannot ignore this issue any more.

drbd is also hitting the same generic_make_request (current->bio_list)
problem, see:
https://www.redhat.com/archives/dm-devel/2016-June/msg00326.html

Mikulas, I've taken your 3 proposed patches patches and refactored them
some to split out intermediate patches that hopefully make review
easier.  Nothing other than variable names and some other style stuff
was changed -- headers were tweaked some to help with clarity.

Please see the 5 topmost "block: ..." patches here:
http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip

It should be noted that Jens had a quick look at this set and wanted to
throw up a little when he saw the (ab)use of a timer to defer punting to
the workqueue.  I explained that without the timer, always punting to
the workqueue, we could hurt performance by reordering IO or crippling
onstack plugging.  He said he'd try to think of a cleaner way forward.

Lars, please feel free to see if this set addresses the similar deadlock
you saw/fixed with drbd.  We need to converge on an acceptable fix for
this problem -- preferably sooner rather than later!

Conversely, Mikulas: if you can easily reproduce the dm-snapshot
deadlock please try Lars' fix to see if it is workable for our DM needs.

Thanks,
Mike

p.s. I'm on holiday until next Wednesday (7/6).. so may be slow to
     respond until then.

Re: [PATCH 0/3] offload bios to a thread

[Mike Snitzer]

[cc'ing linux-block and drbd folks]

On Tue, Jun 28 2016 at  8:16pm -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> Hi
> 
> Here I'm sending three patches to fix the deadlocks in snapshot and 
> snapshot-merge.
> 
> The first patch fixes the deadlock, the following 2 patches introduce a 
> timer, so that bios are not offloaded immediatelly, they are offloaded 
> after a specified timeout, because immediate offloading can change order 
> of bios and it could theoretically produce regressions. I don't know if 
> these regressions really exist or not.
> 
> If there is some way to push the patches upstream, try it.

Some fix must happen before the more recent upstream kernels can be
reliably used in stacked bio-based workloads (in production).  We simply
cannot ignore this issue any more.

drbd is also hitting the same generic_make_request (current->bio_list)
problem, see:
https://www.redhat.com/archives/dm-devel/2016-June/msg00326.html

Mikulas, I've taken your 3 proposed patches patches and refactored them
some to split out intermediate patches that hopefully make review
easier.  Nothing other than variable names and some other style stuff
was changed -- headers were tweaked some to help with clarity.

Please see the 5 topmost "block: ..." patches here:
http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip

It should be noted that Jens had a quick look at this set and wanted to
throw up a little when he saw the (ab)use of a timer to defer punting to
the workqueue.  I explained that without the timer, always punting to
the workqueue, we could hurt performance by reordering IO or crippling
onstack plugging.  He said he'd try to think of a cleaner way forward.

Lars, please feel free to see if this set addresses the similar deadlock
you saw/fixed with drbd.  We need to converge on an acceptable fix for
this problem -- preferably sooner rather than later!

Conversely, Mikulas: if you can easily reproduce the dm-snapshot
deadlock please try Lars' fix to see if it is workable for our DM needs.

Thanks,
Mike

p.s. I'm on holiday until next Wednesday (7/6).. so may be slow to
     respond until then.

Re: [PATCH 0/3] offload bios to a thread

[Mike Snitzer]

On Thu, Jun 30 2016 at  3:40pm -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> [cc'ing linux-block and drbd folks]
> 
> On Tue, Jun 28 2016 at  8:16pm -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > Hi
> > 
> > Here I'm sending three patches to fix the deadlocks in snapshot and 
> > snapshot-merge.
> > 
> > The first patch fixes the deadlock, the following 2 patches introduce a 
> > timer, so that bios are not offloaded immediatelly, they are offloaded 
> > after a specified timeout, because immediate offloading can change order 
> > of bios and it could theoretically produce regressions. I don't know if 
> > these regressions really exist or not.
> > 
> > If there is some way to push the patches upstream, try it.
> 
> Some fix must happen before the more recent upstream kernels can be
> reliably used in stacked bio-based workloads (in production).  We simply
> cannot ignore this issue any more.
> 
> drbd is also hitting the same generic_make_request (current->bio_list)
> problem, see:
> https://www.redhat.com/archives/dm-devel/2016-June/msg00326.html
> 
> Mikulas, I've taken your 3 proposed patches patches and refactored them
> some to split out intermediate patches that hopefully make review
> easier.  Nothing other than variable names and some other style stuff
> was changed -- headers were tweaked some to help with clarity.
> 
> Please see the 5 topmost "block: ..." patches here:
> http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip
> 
> It should be noted that Jens had a quick look at this set and wanted to
> throw up a little when he saw the (ab)use of a timer to defer punting to
> the workqueue.  I explained that without the timer, always punting to
> the workqueue, we could hurt performance by reordering IO or crippling
> onstack plugging.  He said he'd try to think of a cleaner way forward.
> 
> Lars, please feel free to see if this set addresses the similar deadlock
> you saw/fixed with drbd.  We need to converge on an acceptable fix for
> this problem -- preferably sooner rather than later!
> 
> Conversely, Mikulas: if you can easily reproduce the dm-snapshot
> deadlock please try Lars' fix to see if it is workable for our DM needs.

I hadn't reviewed Lars' patch yet but Mikulas pointed out to me that
Lars' patch is focused on the blk_queue_split() path -- and given that
DM doesn't use this function (nor do DM devices even have a 'bio_split'
bioset, see commit dbba42d8a9e) it won't fix the DM (snapshot) deadlock.

Re: [PATCH 0/3] offload bios to a thread

[Mike Snitzer]

On Thu, Jun 30 2016 at  3:40pm -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> [cc'ing linux-block and drbd folks]
> 
> On Tue, Jun 28 2016 at  8:16pm -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > Hi
> > 
> > Here I'm sending three patches to fix the deadlocks in snapshot and 
> > snapshot-merge.
> > 
> > The first patch fixes the deadlock, the following 2 patches introduce a 
> > timer, so that bios are not offloaded immediatelly, they are offloaded 
> > after a specified timeout, because immediate offloading can change order 
> > of bios and it could theoretically produce regressions. I don't know if 
> > these regressions really exist or not.
> > 
> > If there is some way to push the patches upstream, try it.
> 
> Some fix must happen before the more recent upstream kernels can be
> reliably used in stacked bio-based workloads (in production).  We simply
> cannot ignore this issue any more.
> 
> drbd is also hitting the same generic_make_request (current->bio_list)
> problem, see:
> https://www.redhat.com/archives/dm-devel/2016-June/msg00326.html
> 
> Mikulas, I've taken your 3 proposed patches patches and refactored them
> some to split out intermediate patches that hopefully make review
> easier.  Nothing other than variable names and some other style stuff
> was changed -- headers were tweaked some to help with clarity.
> 
> Please see the 5 topmost "block: ..." patches here:
> http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip
> 
> It should be noted that Jens had a quick look at this set and wanted to
> throw up a little when he saw the (ab)use of a timer to defer punting to
> the workqueue.  I explained that without the timer, always punting to
> the workqueue, we could hurt performance by reordering IO or crippling
> onstack plugging.  He said he'd try to think of a cleaner way forward.
> 
> Lars, please feel free to see if this set addresses the similar deadlock
> you saw/fixed with drbd.  We need to converge on an acceptable fix for
> this problem -- preferably sooner rather than later!
> 
> Conversely, Mikulas: if you can easily reproduce the dm-snapshot
> deadlock please try Lars' fix to see if it is workable for our DM needs.

I hadn't reviewed Lars' patch yet but Mikulas pointed out to me that
Lars' patch is focused on the blk_queue_split() path -- and given that
DM doesn't use this function (nor do DM devices even have a 'bio_split'
bioset, see commit dbba42d8a9e) it won't fix the DM (snapshot) deadlock.

Re: [PATCH 0/3] offload bios to a thread

[Lars Ellenberg]

On Thu, Jun 30, 2016 at 07:15:18PM -0400, Mike Snitzer wrote:
> > Lars, please feel free to see if this set addresses the similar deadlock
> > you saw/fixed with drbd.

I'm pretty sure it will help, but will confirm.

> > We need to converge on an acceptable fix for
> > this problem -- preferably sooner rather than later!
> > 
> > Conversely, Mikulas: if you can easily reproduce the dm-snapshot
> > deadlock please try Lars' fix to see if it is workable for our DM needs.
> 
> I hadn't reviewed Lars' patch yet but Mikulas pointed out to me that
> Lars' patch is focused on the blk_queue_split() path -- and given that
> DM doesn't use this function (nor do DM devices even have a 'bio_split'
> bioset, see commit dbba42d8a9e) it won't fix the DM (snapshot) deadlock.

Don't you get it implicitly when using dm-mq -> blk-mq?

    Lars
	

Re: [PATCH 0/3] offload bios to a thread

[Lars Ellenberg]

On Thu, Jun 30, 2016 at 07:15:18PM -0400, Mike Snitzer wrote:
> > Lars, please feel free to see if this set addresses the similar deadlock
> > you saw/fixed with drbd.

I'm pretty sure it will help, but will confirm.

> > We need to converge on an acceptable fix for
> > this problem -- preferably sooner rather than later!
> > 
> > Conversely, Mikulas: if you can easily reproduce the dm-snapshot
> > deadlock please try Lars' fix to see if it is workable for our DM needs.
> 
> I hadn't reviewed Lars' patch yet but Mikulas pointed out to me that
> Lars' patch is focused on the blk_queue_split() path -- and given that
> DM doesn't use this function (nor do DM devices even have a 'bio_split'
> bioset, see commit dbba42d8a9e) it won't fix the DM (snapshot) deadlock.

Don't you get it implicitly when using dm-mq -> blk-mq?

    Lars
	

Re: [PATCH 0/3] offload bios to a thread

[Mikulas Patocka]

On Mon, 4 Jul 2016, Lars Ellenberg wrote:

> On Thu, Jun 30, 2016 at 07:15:18PM -0400, Mike Snitzer wrote:
> > > Lars, please feel free to see if this set addresses the similar deadlock
> > > you saw/fixed with drbd.
> 
> I'm pretty sure it will help, but will confirm.
> 
> > > We need to converge on an acceptable fix for
> > > this problem -- preferably sooner rather than later!
> > > 
> > > Conversely, Mikulas: if you can easily reproduce the dm-snapshot
> > > deadlock please try Lars' fix to see if it is workable for our DM needs.
> > 
> > I hadn't reviewed Lars' patch yet but Mikulas pointed out to me that
> > Lars' patch is focused on the blk_queue_split() path -- and given that
> > DM doesn't use this function (nor do DM devices even have a 'bio_split'
> > bioset, see commit dbba42d8a9e) it won't fix the DM (snapshot) deadlock.
> 
> Don't you get it implicitly when using dm-mq -> blk-mq?
> 
>     Lars

There were observed deadlocks just between dm targets, caused by queuing 
bios on current->bio_list.

The underlying block device was not involved in the deadlocks. Therefore I 
conclude that changing the behavior of blk_queue_split would not resolve 
these deadlocks (because dm targets do not use blk_queue_split).

Mikulas

Re: [PATCH 0/3] offload bios to a thread

[Mikulas Patocka]

On Mon, 4 Jul 2016, Lars Ellenberg wrote:

> On Thu, Jun 30, 2016 at 07:15:18PM -0400, Mike Snitzer wrote:
> > > Lars, please feel free to see if this set addresses the similar deadlock
> > > you saw/fixed with drbd.
> 
> I'm pretty sure it will help, but will confirm.
> 
> > > We need to converge on an acceptable fix for
> > > this problem -- preferably sooner rather than later!
> > > 
> > > Conversely, Mikulas: if you can easily reproduce the dm-snapshot
> > > deadlock please try Lars' fix to see if it is workable for our DM needs.
> > 
> > I hadn't reviewed Lars' patch yet but Mikulas pointed out to me that
> > Lars' patch is focused on the blk_queue_split() path -- and given that
> > DM doesn't use this function (nor do DM devices even have a 'bio_split'
> > bioset, see commit dbba42d8a9e) it won't fix the DM (snapshot) deadlock.
> 
> Don't you get it implicitly when using dm-mq -> blk-mq?
> 
>     Lars

There were observed deadlocks just between dm targets, caused by queuing 
bios on current->bio_list.

The underlying block device was not involved in the deadlocks. Therefore I 
conclude that changing the behavior of blk_queue_split would not resolve 
these deadlocks (because dm targets do not use blk_queue_split).

Mikulas

Re: [PATCH 0/3] offload bios to a thread

[Mikulas Patocka]

On Thu, 30 Jun 2016, Mike Snitzer wrote:

> [cc'ing linux-block and drbd folks]
> 
> On Tue, Jun 28 2016 at  8:16pm -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > Hi
> > 
> > Here I'm sending three patches to fix the deadlocks in snapshot and 
> > snapshot-merge.
> > 
> > The first patch fixes the deadlock, the following 2 patches introduce a 
> > timer, so that bios are not offloaded immediatelly, they are offloaded 
> > after a specified timeout, because immediate offloading can change order 
> > of bios and it could theoretically produce regressions. I don't know if 
> > these regressions really exist or not.
> > 
> > If there is some way to push the patches upstream, try it.
> 
> Some fix must happen before the more recent upstream kernels can be
> reliably used in stacked bio-based workloads (in production).  We simply
> cannot ignore this issue any more.
> 
> drbd is also hitting the same generic_make_request (current->bio_list)
> problem, see:
> https://www.redhat.com/archives/dm-devel/2016-June/msg00326.html
> 
> Mikulas, I've taken your 3 proposed patches patches and refactored them
> some to split out intermediate patches that hopefully make review
> easier.  Nothing other than variable names and some other style stuff
> was changed -- headers were tweaked some to help with clarity.
> 
> Please see the 5 topmost "block: ..." patches here:
> http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip

I found a problem with the patches when using loop device - we must not 
offload bios to the rescue thread if they are allocated from fs_bio_set. 
I'll send a second version of the patches with this change. You can 
incorporate that change to your git tree.

> It should be noted that Jens had a quick look at this set and wanted to
> throw up a little when he saw the (ab)use of a timer to defer punting to
> the workqueue.  I explained that without the timer, always punting to
> the workqueue, we could hurt performance by reordering IO or crippling
> onstack plugging.  He said he'd try to think of a cleaner way forward.

The behavior depends on the timer only in a situation when the deadlock 
actually happens - the timer doesn't hurt performance on normal use. So, 
it's better to have timed delay in bio processing than a deadlock :)

The timer part can be dropped entirely if someone shows that offloading 
bios on schedule doesn't hurt performance in any way. Does anyone have a 
large collection of block layer performance tests that could be tried to 
detect if the regression happens?

Mikulas

Re: [PATCH 0/3] offload bios to a thread

[Mikulas Patocka]

On Thu, 30 Jun 2016, Mike Snitzer wrote:

> [cc'ing linux-block and drbd folks]
> 
> On Tue, Jun 28 2016 at  8:16pm -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > Hi
> > 
> > Here I'm sending three patches to fix the deadlocks in snapshot and 
> > snapshot-merge.
> > 
> > The first patch fixes the deadlock, the following 2 patches introduce a 
> > timer, so that bios are not offloaded immediatelly, they are offloaded 
> > after a specified timeout, because immediate offloading can change order 
> > of bios and it could theoretically produce regressions. I don't know if 
> > these regressions really exist or not.
> > 
> > If there is some way to push the patches upstream, try it.
> 
> Some fix must happen before the more recent upstream kernels can be
> reliably used in stacked bio-based workloads (in production).  We simply
> cannot ignore this issue any more.
> 
> drbd is also hitting the same generic_make_request (current->bio_list)
> problem, see:
> https://www.redhat.com/archives/dm-devel/2016-June/msg00326.html
> 
> Mikulas, I've taken your 3 proposed patches patches and refactored them
> some to split out intermediate patches that hopefully make review
> easier.  Nothing other than variable names and some other style stuff
> was changed -- headers were tweaked some to help with clarity.
> 
> Please see the 5 topmost "block: ..." patches here:
> http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip

I found a problem with the patches when using loop device - we must not 
offload bios to the rescue thread if they are allocated from fs_bio_set. 
I'll send a second version of the patches with this change. You can 
incorporate that change to your git tree.

> It should be noted that Jens had a quick look at this set and wanted to
> throw up a little when he saw the (ab)use of a timer to defer punting to
> the workqueue.  I explained that without the timer, always punting to
> the workqueue, we could hurt performance by reordering IO or crippling
> onstack plugging.  He said he'd try to think of a cleaner way forward.

The behavior depends on the timer only in a situation when the deadlock 
actually happens - the timer doesn't hurt performance on normal use. So, 
it's better to have timed delay in bio processing than a deadlock :)

The timer part can be dropped entirely if someone shows that offloading 
bios on schedule doesn't hurt performance in any way. Does anyone have a 
large collection of block layer performance tests that could be tried to 
detect if the regression happens?

Mikulas

Re: [PATCH 0/3] offload bios to a thread

[Mikulas Patocka]

On Wed, 6 Jul 2016, Mike Snitzer wrote:

> > > I'd much preferred you just send an incremental fix that built on the
> > > tree you know I started, here:
> > > http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip
> > 
> > You need to change three patches in your git:
> > * block: flush queued bios when process blocks to avoid deadlock
> > * block: prepare for timed offload of queued bios to workqueue
> > * block: use timed offload of queued bios to a workqueue
> > because this bug is present in all of them.
> > 
> > When these patches are sent to Linus, the bug should not be present in any 
> > of them.
> 
> Yes, I'm aware.  Please review:
> http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip

Yes, It's OK.

Mikulas

Re: [PATCH 0/3] offload bios to a thread

[Mike Snitzer]

On Wed, Jul 06 2016 at  9:53am -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> 
> 
> On Wed, 6 Jul 2016, Mike Snitzer wrote:
> 
> > On Mon, Jul 04 2016 at  6:53pm -0400,
> > Mikulas Patocka <mpatocka@redhat.com> wrote:
> > 
> > > Hi
> > > 
> > > This is the second version of patches that fix deadlocks by redirecting 
> > > bios from current->bio_list to rescuer workqueues.
> > > 
> > > I found out that the original patches caused deadlock with the loopback 
> > > device. When the loopback device is used, both lower and upper filesystems 
> > > use the same bio set - fs_bio_set. Consequently, bios submitted by both of 
> > > them end up on the same rescuer workqueue. There is a deadlock possibility 
> > > - if generic_make_request for the upper filesystem's bio blocks (because 
> > > there are too many requests in flight on the loop device), it may stall 
> > > processing some bios for the lower filesystem.
> > > 
> > > Ideadlly, each filesystem should have its own bio set. But it doesn't. So 
> > > I fix this problem by not offloading bios allocated from fs_bio_set.
> > 
> > I'd much preferred you just send an incremental fix that built on the
> > tree you know I started, here:
> > http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip
> 
> You need to change three patches in your git:
> * block: flush queued bios when process blocks to avoid deadlock
> * block: prepare for timed offload of queued bios to workqueue
> * block: use timed offload of queued bios to a workqueue
> because this bug is present in all of them.
> 
> When these patches are sent to Linus, the bug should not be present in any 
> of them.

Yes, I'm aware.  Please review:
http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip

Re: [PATCH 0/3] offload bios to a thread

[Mikulas Patocka]

On Wed, 6 Jul 2016, Mike Snitzer wrote:

> On Mon, Jul 04 2016 at  6:53pm -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > Hi
> > 
> > This is the second version of patches that fix deadlocks by redirecting 
> > bios from current->bio_list to rescuer workqueues.
> > 
> > I found out that the original patches caused deadlock with the loopback 
> > device. When the loopback device is used, both lower and upper filesystems 
> > use the same bio set - fs_bio_set. Consequently, bios submitted by both of 
> > them end up on the same rescuer workqueue. There is a deadlock possibility 
> > - if generic_make_request for the upper filesystem's bio blocks (because 
> > there are too many requests in flight on the loop device), it may stall 
> > processing some bios for the lower filesystem.
> > 
> > Ideadlly, each filesystem should have its own bio set. But it doesn't. So 
> > I fix this problem by not offloading bios allocated from fs_bio_set.
> 
> I'd much preferred you just send an incremental fix that built on the
> tree you know I started, here:
> http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip

You need to change three patches in your git:
* block: flush queued bios when process blocks to avoid deadlock
* block: prepare for timed offload of queued bios to workqueue
* block: use timed offload of queued bios to a workqueue
because this bug is present in all of them.

When these patches are sent to Linus, the bug should not be present in any 
of them.

Mikulas

> I've now folded your fix into this tree.
> 
> But please don't ignore work you know that was done to further prepare
> your patches for inclusion.  It makes for tedious busy work on my end to
> pull out the incremental fix, which is simply:
> 
> diff --git a/block/bio.c b/block/bio.c
> index 7c49b91..80ebe88 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -357,7 +357,9 @@ static void bio_alloc_rescue(struct work_struct *work)
>   * to their rescue workqueue.
>   *
>   * If the bio doesn't have a bio_set, we leave it on queued_bios->bio_list.
> - * However, stacking drivers should use bio_set, so this shouldn't be
> + * If the bio is allocated from fs_bio_set, we must leave it to avoid
> + * deadlock on loopback block device.
> + * But stacking drivers should use a bio_set, so this shouldn't be
>   * an issue.
>   */
>  static void blk_timer_flush_bio_list(unsigned long data)
> @@ -371,7 +373,7 @@ static void blk_timer_flush_bio_list(unsigned long data)
>  	while ((bio = bio_list_pop(&list))) {
>  		unsigned long flags;
>  		struct bio_set *bs = bio->bi_pool;
> -		if (unlikely(!bs)) {
> +		if (unlikely(!bs) || bs == fs_bio_set) {
>  			bio_list_add(&queued_bios->bio_list, bio);
>  			continue;
>  		}
> 

Re: [PATCH 0/3] offload bios to a thread

[Mike Snitzer]

On Mon, Jul 04 2016 at  6:53pm -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> Hi
> 
> This is the second version of patches that fix deadlocks by redirecting 
> bios from current->bio_list to rescuer workqueues.
> 
> I found out that the original patches caused deadlock with the loopback 
> device. When the loopback device is used, both lower and upper filesystems 
> use the same bio set - fs_bio_set. Consequently, bios submitted by both of 
> them end up on the same rescuer workqueue. There is a deadlock possibility 
> - if generic_make_request for the upper filesystem's bio blocks (because 
> there are too many requests in flight on the loop device), it may stall 
> processing some bios for the lower filesystem.
> 
> Ideadlly, each filesystem should have its own bio set. But it doesn't. So 
> I fix this problem by not offloading bios allocated from fs_bio_set.

I'd much preferred you just send an incremental fix that built on the
tree you know I started, here:
http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip

I've now folded your fix into this tree.

But please don't ignore work you know that was done to further prepare
your patches for inclusion.  It makes for tedious busy work on my end to
pull out the incremental fix, which is simply:

diff --git a/block/bio.c b/block/bio.c
index 7c49b91..80ebe88 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -357,7 +357,9 @@ static void bio_alloc_rescue(struct work_struct *work)
  * to their rescue workqueue.
  *
  * If the bio doesn't have a bio_set, we leave it on queued_bios->bio_list.
- * However, stacking drivers should use bio_set, so this shouldn't be
+ * If the bio is allocated from fs_bio_set, we must leave it to avoid
+ * deadlock on loopback block device.
+ * But stacking drivers should use a bio_set, so this shouldn't be
  * an issue.
  */
 static void blk_timer_flush_bio_list(unsigned long data)
@@ -371,7 +373,7 @@ static void blk_timer_flush_bio_list(unsigned long data)
 	while ((bio = bio_list_pop(&list))) {
 		unsigned long flags;
 		struct bio_set *bs = bio->bi_pool;
-		if (unlikely(!bs)) {
+		if (unlikely(!bs) || bs == fs_bio_set) {
 			bio_list_add(&queued_bios->bio_list, bio);
 			continue;
 		}

[PATCH 0/3] offload bios to a thread

[Mikulas Patocka]

Hi

This is the second version of patches that fix deadlocks by redirecting 
bios from current->bio_list to rescuer workqueues.

I found out that the original patches caused deadlock with the loopback 
device. When the loopback device is used, both lower and upper filesystems 
use the same bio set - fs_bio_set. Consequently, bios submitted by both of 
them end up on the same rescuer workqueue. There is a deadlock possibility 
- if generic_make_request for the upper filesystem's bio blocks (because 
there are too many requests in flight on the loop device), it may stall 
processing some bios for the lower filesystem.

Ideadlly, each filesystem should have its own bio set. But it doesn't. So 
I fix this problem by not offloading bios allocated from fs_bio_set.

Mikulas