On Wed, 6 Jul 2016, Mike Snitzer wrote: > On Mon, Jul 04 2016 at 6:53pm -0400, > Mikulas Patocka <mpatocka@xxxxxxxxxx> 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; > } > -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
