Linux Device Mapper development
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: Mike Snitzer <snitzer@redhat.com>
Cc: Jens Axboe <jaxboe@fusionio.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"hch@infradead.org" <hch@infradead.org>,
	dm-devel@redhat.com
Subject: Re: [PATCH 05/10] block: remove per-queue plugging
Date: Tue, 5 Apr 2011 13:05:41 +1000	[thread overview]
Message-ID: <20110405130541.6c2b5f86@notabene.brown> (raw)
In-Reply-To: <20110310005810.GA17911@redhat.com>

On Wed, 9 Mar 2011 19:58:10 -0500 Mike Snitzer <snitzer@redhat.com> wrote:

> Also, in your MD changes, you removed all calls to md_unplug() but
> didn't remove md_unplug().  Seems it should be removed along with the
> 'plug' member of 'struct mddev_t'?  Neil?

I've been distracted by other things and only just managed to have a look at
this.

The new plugging code seems to completely ignore the needs of stacked devices
- or at least my needs in md.

For RAID1 with a write-intent-bitmap, I queue all write requests and then on
an unplug I update the write-intent-bitmap to mark all the relevant blocks
and then release the writes.

With the new code there is no way for an unplug event to wake up the raid1d
thread to start the writeout - I haven't tested it but I suspect it will just
hang.

Similarly for RAID5 I gather write bios (long before they become 'struct
request' which is what the plugging code understands) and on an unplug event
I release the writes - hopefully with enough bios per stripe so that we don't
need to pre-read.

Possibly the simplest fix would be to have a second list_head in 'struct
blk_plug' which contained callbacks (a function pointer a list_head in a
struct which is passed as an arg to the function!).
blk_finish_plug could then walk the list and call the call-backs.
It would be quite easy to hook into that.


I suspect I also need to add blk_start_plug/blk_finish_plug around the loop
in raid1d/raid5d/raid10d, but that is pretty straight forward.

Am I missing something important?
Is there a better way to get an unplug event to md?

Thanks,
NeilBrown

  reply	other threads:[~2011-04-05  3:05 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1295659049-2688-1-git-send-email-jaxboe@fusionio.com>
     [not found] ` <1295659049-2688-6-git-send-email-jaxboe@fusionio.com>
     [not found]   ` <AANLkTin8FoXX6oqUyW+scwhadyX-TfW16_oKjvngU9-m@mail.gmail.com>
     [not found]     ` <20110303221353.GA10366@redhat.com>
     [not found]       ` <4D761E0D.8050200@fusionio.com>
     [not found]         ` <20110308202100.GA31744@redhat.com>
     [not found]           ` <4D76912C.9040705@fusionio.com>
     [not found]             ` <20110308220526.GA393@redhat.com>
2011-03-10  0:58               ` [PATCH 05/10] block: remove per-queue plugging Mike Snitzer
2011-04-05  3:05                 ` NeilBrown [this message]
2011-04-11  4:50                   ` NeilBrown
2011-04-11  9:19                     ` Jens Axboe
2011-04-11 10:59                       ` NeilBrown
2011-04-11 11:04                         ` Jens Axboe
2011-04-11 11:26                           ` NeilBrown
2011-04-11 11:37                             ` Jens Axboe
2011-04-11 12:05                               ` NeilBrown
2011-04-11 12:11                                 ` Jens Axboe
2011-04-11 12:36                                   ` NeilBrown
2011-04-11 12:48                                     ` Jens Axboe
2011-04-12  1:12                                       ` hch
2011-04-12  8:36                                         ` Jens Axboe
2011-04-12 12:22                                           ` Dave Chinner
2011-04-12 12:28                                             ` Jens Axboe
2011-04-12 12:41                                               ` Dave Chinner
2011-04-12 12:58                                                 ` Jens Axboe
2011-04-12 13:31                                                   ` Dave Chinner
2011-04-12 13:45                                                     ` Jens Axboe
2011-04-12 14:34                                                       ` Dave Chinner
2011-04-12 21:08                                                         ` NeilBrown
2011-04-13  2:23                                                           ` Linus Torvalds
2011-04-13 11:12                                                             ` Peter Zijlstra
2011-04-13 11:23                                                               ` Jens Axboe
2011-04-13 11:41                                                                 ` Peter Zijlstra
2011-04-13 15:13                                                                 ` Linus Torvalds
2011-04-13 17:35                                                                   ` Jens Axboe
2011-04-12 16:58                                                     ` hch
2011-04-12 17:29                                                       ` Jens Axboe
2011-04-12 16:44                                                   ` hch
2011-04-12 16:49                                                     ` Jens Axboe
2011-04-12 16:54                                                       ` hch
2011-04-12 17:24                                                         ` Jens Axboe
2011-04-12 13:40                                               ` Dave Chinner
2011-04-12 13:48                                                 ` Jens Axboe
2011-04-12 23:35                                                   ` Dave Chinner
2011-04-12 16:50                                           ` hch
2011-04-15  4:26                                   ` hch
2011-04-15  6:34                                     ` Jens Axboe
2011-04-17 22:19                                   ` NeilBrown
2011-04-18  4:19                                     ` NeilBrown
2011-04-18  6:38                                     ` Jens Axboe
2011-04-18  7:25                                       ` NeilBrown
2011-04-18  8:10                                         ` Jens Axboe
2011-04-18  8:33                                           ` NeilBrown
2011-04-18  8:42                                             ` Jens Axboe
2011-04-18 21:23                                             ` hch
2011-04-18 21:30                                             ` hch
2011-04-18 22:38                                               ` NeilBrown
2011-04-20 10:55                                                 ` hch
2011-04-18  9:19                                           ` hch
2011-04-18  9:40                                             ` [dm-devel] " Hannes Reinecke
2011-04-18  9:47                                               ` Jens Axboe
2011-04-18  9:46                                             ` Jens Axboe
2011-04-11 11:55                         ` NeilBrown
2011-04-11 12:12                           ` Jens Axboe
2011-04-11 22:58                             ` hch
2011-04-12  6:20                               ` Jens Axboe
2011-04-11 16:59                     ` hch
2011-04-11 21:14                       ` NeilBrown
2011-04-11 22:59                         ` hch
2011-04-12  6:18                         ` Jens Axboe

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=20110405130541.6c2b5f86@notabene.brown \
    --to=neilb@suse.de \
    --cc=dm-devel@redhat.com \
    --cc=hch@infradead.org \
    --cc=jaxboe@fusionio.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=snitzer@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox