All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alasdair G Kergon <agk@redhat.com>
To: Kent Overstreet <koverstreet@google.com>
Cc: Vivek Goyal <vgoyal@redhat.com>, Jens Axboe <axboe@kernel.dk>,
	dm-devel@redhat.com, linux-kernel@vger.kernel.org,
	linux-bcache@vger.kernel.org, mpatocka@redhat.com,
	bharrosh@panasas.com, Tejun Heo <tj@kernel.org>
Subject: Re: [dm-devel] [PATCH v7 9/9] block: Avoid deadlocks with bio allocation by stacking drivers
Date: Wed, 29 Aug 2012 15:39:14 +0100	[thread overview]
Message-ID: <20120829143913.GA5500@agk-dp.fab.redhat.com> (raw)
In-Reply-To: <20120829125759.GB12504@redhat.com>

On Wed, Aug 29, 2012 at 08:57:59AM -0400, Vivek Goyal wrote:
> I would say keep all the bio splitting patches and any fixes w.r.t
> deadlocks in a seprate series. As this is little complicated and a lot
> of is just theoritical corner cases. If you limit this series to just
> bio_set related cleanups, it becomes more acceptable for inclusion.
 
Yes, please keep the splitting patches separate.  The current code gets
away with what it does through statistics making the deadlocks very
unlikely.

It's also instructive to remember why the code is the way it is: it used
to process bios for underlying devices immediately, but this sometimes
meant too much recursive stack growth.  If a per-device rescuer thread
is to be made available (as well as the mempool), the option of
reinstating recursion is there too - only punting to workqueue when the
stack actually becomes "too big".  (Also bear in mind that some dm
targets may have dependencies on their own mempools - submission can
block there too.)  I find it helpful only to consider splitting into two
pieces - it must always be possible to process the first piece (i.e.
process it at the next layer down in the stack) and complete it
independently of what happens to the second piece (which might require
further splitting and block until the first piece has completed).

Alasdair


WARNING: multiple messages have this Message-ID (diff)
From: Alasdair G Kergon <agk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: Kent Overstreet <koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Cc: Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	Jens Axboe <axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-bcache-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	mpatocka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	bharrosh-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org,
	Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Subject: Re: [dm-devel] [PATCH v7 9/9] block: Avoid deadlocks with bio allocation by stacking drivers
Date: Wed, 29 Aug 2012 15:39:14 +0100	[thread overview]
Message-ID: <20120829143913.GA5500@agk-dp.fab.redhat.com> (raw)
In-Reply-To: <20120829125759.GB12504-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Wed, Aug 29, 2012 at 08:57:59AM -0400, Vivek Goyal wrote:
> I would say keep all the bio splitting patches and any fixes w.r.t
> deadlocks in a seprate series. As this is little complicated and a lot
> of is just theoritical corner cases. If you limit this series to just
> bio_set related cleanups, it becomes more acceptable for inclusion.
 
Yes, please keep the splitting patches separate.  The current code gets
away with what it does through statistics making the deadlocks very
unlikely.

It's also instructive to remember why the code is the way it is: it used
to process bios for underlying devices immediately, but this sometimes
meant too much recursive stack growth.  If a per-device rescuer thread
is to be made available (as well as the mempool), the option of
reinstating recursion is there too - only punting to workqueue when the
stack actually becomes "too big".  (Also bear in mind that some dm
targets may have dependencies on their own mempools - submission can
block there too.)  I find it helpful only to consider splitting into two
pieces - it must always be possible to process the first piece (i.e.
process it at the next layer down in the stack) and complete it
independently of what happens to the second piece (which might require
further splitting and block until the first piece has completed).

Alasdair

  reply	other threads:[~2012-08-29 14:39 UTC|newest]

Thread overview: 128+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-28 17:37 [PATCH v7 0/9] Block cleanups, deadlock fix Kent Overstreet
2012-08-28 17:37 ` [PATCH v7 1/9] block: Generalized bio pool freeing Kent Overstreet
2012-08-28 17:37   ` Kent Overstreet
2012-08-28 17:37 ` [PATCH v7 2/9] dm: Use bioset's front_pad for dm_rq_clone_bio_info Kent Overstreet
2012-08-28 17:37 ` [PATCH v7 3/9] block: Add bio_reset() Kent Overstreet
2012-08-28 17:37   ` Kent Overstreet
2012-08-28 20:31   ` Tejun Heo
2012-08-28 20:31     ` Tejun Heo
2012-08-28 22:17     ` Kent Overstreet
2012-08-28 22:53       ` Kent Overstreet
2012-08-28 22:53         ` Kent Overstreet
2012-09-01  2:23       ` Tejun Heo
2012-09-01  2:23         ` Tejun Heo
2012-09-05 20:13         ` Kent Overstreet
2012-08-28 17:37 ` [PATCH v7 4/9] pktcdvd: Switch to bio_kmalloc() Kent Overstreet
2012-08-28 20:32   ` Tejun Heo
2012-08-28 20:32     ` Tejun Heo
2012-08-28 22:24     ` Kent Overstreet
2012-09-04  9:05     ` Jiri Kosina
2012-09-04  9:05       ` Jiri Kosina
2012-09-05 19:44       ` Kent Overstreet
2012-09-05 19:44         ` Kent Overstreet
2012-08-28 17:37 ` [PATCH v7 5/9] block: Kill bi_destructor Kent Overstreet
2012-08-28 20:36   ` Tejun Heo
2012-08-28 20:36     ` Tejun Heo
2012-08-28 22:07     ` Kent Overstreet
2012-08-28 17:37 ` [PATCH v7 6/9] block: Consolidate bio_alloc_bioset(), bio_kmalloc() Kent Overstreet
2012-08-28 17:37   ` Kent Overstreet
2012-08-28 20:41   ` Tejun Heo
2012-08-28 20:41     ` Tejun Heo
2012-08-28 22:03     ` Kent Overstreet
2012-08-28 22:03       ` Kent Overstreet
2012-09-01  2:17       ` Tejun Heo
2012-08-28 17:37 ` [PATCH v7 7/9] block: Add bio_clone_bioset(), bio_clone_kmalloc() Kent Overstreet
2012-08-28 20:44   ` Tejun Heo
2012-08-28 20:44     ` Tejun Heo
2012-08-28 22:05     ` Kent Overstreet
2012-09-01  2:19       ` Tejun Heo
2012-09-01  2:19         ` Tejun Heo
2012-08-28 17:37 ` [PATCH v7 8/9] block: Reorder struct bio_set Kent Overstreet
2012-08-28 17:37 ` [PATCH v7 9/9] block: Avoid deadlocks with bio allocation by stacking drivers Kent Overstreet
2012-08-28 20:49   ` Tejun Heo
2012-08-28 20:49     ` Tejun Heo
2012-08-28 22:28     ` Kent Overstreet
2012-08-28 23:01       ` Kent Overstreet
2012-08-28 23:01         ` Kent Overstreet
2012-08-29  1:31         ` Vivek Goyal
2012-08-29  1:31           ` Vivek Goyal
2012-08-29  3:25           ` Kent Overstreet
2012-08-29 12:57             ` Vivek Goyal
2012-08-29 12:57               ` Vivek Goyal
2012-08-29 14:39               ` Alasdair G Kergon [this message]
2012-08-29 14:39                 ` [dm-devel] " Alasdair G Kergon
2012-08-29 16:26                 ` Kent Overstreet
2012-08-29 16:26                   ` Kent Overstreet
2012-08-29 21:01                   ` John Stoffel
2012-08-29 21:08                     ` Kent Overstreet
2012-08-29 21:08                       ` Kent Overstreet
2012-08-28 22:06   ` Vivek Goyal
2012-08-28 22:06     ` Vivek Goyal
2012-08-28 22:23     ` Kent Overstreet
2012-08-28 22:23       ` Kent Overstreet
2012-08-29 16:24   ` Mikulas Patocka
2012-08-29 16:50     ` Kent Overstreet
2012-08-29 16:57       ` [dm-devel] " Alasdair G Kergon
2012-08-29 16:57         ` Alasdair G Kergon
2012-08-29 17:07       ` Vivek Goyal
2012-08-29 17:07         ` Vivek Goyal
2012-08-29 17:13         ` Kent Overstreet
2012-08-29 17:13           ` Kent Overstreet
2012-08-29 17:23           ` [dm-devel] " Alasdair G Kergon
2012-08-29 17:23             ` Alasdair G Kergon
2012-08-29 17:32             ` Kent Overstreet
2012-08-30 22:07           ` Vivek Goyal
2012-08-30 22:07             ` Vivek Goyal
2012-08-31  1:43             ` Kent Overstreet
2012-08-31  1:43               ` Kent Overstreet
2012-08-31  1:55               ` Kent Overstreet
2012-08-31  1:55                 ` Kent Overstreet
2012-08-31 15:01               ` Vivek Goyal
2012-09-03  1:26                 ` Kent Overstreet
2012-09-03  1:26                   ` Kent Overstreet
2012-09-03 20:41               ` Mikulas Patocka
2012-09-03 20:41                 ` Mikulas Patocka
2012-09-04  3:41                 ` Kent Overstreet
2012-09-04  3:41                   ` Kent Overstreet
2012-09-04 18:55                   ` Tejun Heo
2012-09-04 18:55                     ` Tejun Heo
2012-09-04 19:01                     ` Tejun Heo
2012-09-04 19:43                       ` Kent Overstreet
2012-09-04 19:43                         ` Kent Overstreet
2012-09-04 19:42                     ` Kent Overstreet
2012-09-04 21:03                       ` Tejun Heo
2012-09-04 21:03                         ` Tejun Heo
2012-09-04 19:26                   ` Mikulas Patocka
2012-09-04 19:26                     ` Mikulas Patocka
2012-09-04 19:39                     ` Vivek Goyal
2012-09-04 19:39                       ` Vivek Goyal
2012-09-04 19:51                     ` [PATCH] dm: Use bioset's front_pad for dm_target_io Kent Overstreet
2012-09-04 19:51                       ` Kent Overstreet
2012-09-04 21:20                       ` Tejun Heo
2012-09-04 21:20                         ` Tejun Heo
2012-09-11 19:28                       ` [PATCH 2] " Mikulas Patocka
2012-09-11 19:28                         ` Mikulas Patocka
2012-09-11 19:50                         ` Kent Overstreet
2012-09-11 19:50                           ` Kent Overstreet
2012-09-12 22:31                           ` Mikulas Patocka
2012-09-12 22:31                             ` Mikulas Patocka
2012-09-14 23:09                             ` [dm-devel] " Alasdair G Kergon
2012-09-14 23:09                               ` Alasdair G Kergon
2012-09-01  2:13             ` [PATCH v7 9/9] block: Avoid deadlocks with bio allocation by stacking drivers Tejun Heo
2012-09-01  2:13               ` Tejun Heo
2012-09-03  1:34               ` [PATCH v2] " Kent Overstreet
2012-09-03  1:34                 ` Kent Overstreet
2012-09-04 15:00               ` [PATCH v7 9/9] " Vivek Goyal
2012-09-04 15:00                 ` Vivek Goyal
2012-09-03  0:49             ` Dave Chinner
2012-09-03  0:49               ` Dave Chinner
2012-09-03  1:17               ` Kent Overstreet
2012-09-03  1:17                 ` Kent Overstreet
2012-09-04 13:54               ` Vivek Goyal
2012-09-04 13:54                 ` Vivek Goyal
2012-09-04 18:26                 ` Tejun Heo
2012-09-04 18:26                   ` Tejun Heo
2012-09-05  3:57                   ` Dave Chinner
2012-09-05  3:57                     ` Dave Chinner
2012-09-05  4:37                     ` Tejun Heo
2012-09-05  4:37                       ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120829143913.GA5500@agk-dp.fab.redhat.com \
    --to=agk@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=bharrosh@panasas.com \
    --cc=dm-devel@redhat.com \
    --cc=koverstreet@google.com \
    --cc=linux-bcache@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpatocka@redhat.com \
    --cc=tj@kernel.org \
    --cc=vgoyal@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.