All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@redhat.com>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>, Kent Overstreet <kmo@daterainc.com>,
	"Alasdair G. Kergon" <agk@redhat.com>,
	linux-kernel@vger.kernel.org, dm-devel@redhat.com
Subject: Re: block: flush queued bios when the process blocks
Date: Tue, 6 Oct 2015 09:47:53 -0400	[thread overview]
Message-ID: <20151006134753.GA30555@redhat.com> (raw)
In-Reply-To: <alpine.LRH.2.02.1510060919120.23547@file01.intranet.prod.int.rdu2.redhat.com>

On Tue, Oct 06 2015 at  9:28am -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> 
> 
> On Mon, 5 Oct 2015, Mike Snitzer wrote:
> 
> > Mikulas,
> > 
> > Could it be that cond_resched() wasn't unplugging?  As was
> > recently raised in this thread: https://lkml.org/lkml/2015/9/18/378
> > Chris Mason's patch from that thread fixed this	issue... I _think_ Linus
> > has since committed Chris' work but I haven't kept my finger on the
> > pulse of that issue.
> 
> I think it doesn't matter (regarding correctness) if cond_reched unplugs 
> on not. If it didn't unplug, the process will be scheduled later, and it 
> will eventually reach the point where it unplugs.

Couldn't the original deadlock you fixed (as described in your first
patch) manifest when a new process is scheduled?
 
> > FYI, I've put rebased versions of your 2 patches in my wip branch, see:
> > http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip
> > 
> > I tweaked the 2nd patch that adds bio_list to plug so that
> > generic_make_request's checks for in_generic_make_request isn't racey
> > (your original patch could happen to have current-plug set but
> > in_generic_make_request not yet set).
> 
> I don't recommend that second patch 
> (http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/commit/?h=wip&id=5e740c2e45a767d8d6ef8ca36b0db705ef6259c4). 
> The patch just complicates things without adding any value. It's also not 
> correct because it plugs bios at places when bios aren't supposed to be 
> plugged

Avoiding another hook the the scheduler is a requirement (from Jens).
"without adding any value": it offers a different strategy for recording
bios to the bio_list by making it part of the plug.  The plug doesn't
actually block bios like requests are plugged.  What am I missing?

  reply	other threads:[~2015-10-06 13:47 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-27 15:03 [PATCH] block: flush queued bios when the process blocks Mikulas Patocka
2014-05-27 15:08 ` Jens Axboe
2014-05-27 15:23   ` Mikulas Patocka
2014-05-27 15:42     ` Jens Axboe
2014-05-27 16:26       ` Mikulas Patocka
2014-05-27 17:33         ` Mike Snitzer
2014-05-27 19:56           ` Kent Overstreet
2015-10-05 19:50             ` Mike Snitzer
2014-05-27 17:42         ` [PATCH] " Jens Axboe
2014-05-27 18:14           ` [dm-devel] " Christoph Hellwig
2014-05-27 19:59             ` Kent Overstreet
2014-05-27 19:56           ` Mikulas Patocka
2014-05-27 20:06             ` Kent Overstreet
2014-05-29 23:52           ` Mikulas Patocka
2015-10-05 20:59             ` Mike Snitzer
2015-10-06 13:28               ` Mikulas Patocka
2015-10-06 13:47                 ` Mike Snitzer [this message]
2015-10-06 14:10                   ` Mikulas Patocka
2015-10-06 14:26                   ` Mikulas Patocka
2015-10-06 18:17               ` [dm-devel] " Mikulas Patocka
2015-10-06 18:50                 ` Mike Snitzer
2015-10-06 20:16                   ` [PATCH v2] " Mike Snitzer
2015-10-06 20:26                     ` Mike Snitzer
2015-10-08 15:04                     ` Mikulas Patocka
2015-10-08 15:08                       ` Mike Snitzer
2015-10-09 19:52                         ` Mike Snitzer
2015-10-09 19:59                           ` Mike Snitzer
2015-10-14 20:47                             ` [PATCH v3 for-4.4] block: flush queued bios when process blocks to avoid deadlock Mike Snitzer
2015-10-14 21:44                               ` Jeff Moyer
2015-10-17 16:04                               ` Ming Lei
2015-10-20 19:57                                 ` Mike Snitzer
2015-10-20 20:03                                 ` Mikulas Patocka
2015-10-21 16:38                                   ` Ming Lei
2015-10-21 21:49                                     ` Mikulas Patocka
2015-10-22  1:53                                       ` Ming Lei
2015-10-15  3:27                           ` [PATCH v2] block: flush queued bios when the process blocks Ming Lei
2015-10-15  8:06                             ` Mike Snitzer
2015-10-16  3:08                               ` Ming Lei
2015-10-16 15:29                                 ` Mike Snitzer
2015-10-17 15:54                                   ` Ming Lei
2015-10-17 15:54                                     ` Ming Lei
2015-10-09 11:58                     ` kbuild test robot
2014-05-27 17:59   ` [PATCH] " Kent Overstreet

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=20151006134753.GA30555@redhat.com \
    --to=snitzer@redhat.com \
    --cc=agk@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=dm-devel@redhat.com \
    --cc=kmo@daterainc.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpatocka@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.