linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Tejun Heo <tj@kernel.org>
Cc: Vivek Goyal <vgoyal@redhat.com>,
	Shaohua Li <shaohua.li@intel.com>,
	linux kernel mailing list <linux-kernel@vger.kernel.org>,
	koverstreet@google.com, Neil Brown <neilb@suse.de>
Subject: Re: Kernel crash in icq_free_icq_rcu
Date: Wed, 18 Jan 2012 20:07:31 +0100	[thread overview]
Message-ID: <4F171873.9040308@kernel.dk> (raw)
In-Reply-To: <20120118171029.GD30664@google.com>

On 2012-01-18 18:10, Tejun Heo wrote:
> Hello, Vivek.
> 
> On Wed, Jan 18, 2012 at 11:36:38AM -0500, Vivek Goyal wrote:
>> Not calling ioscheduler during plug merge will allow merging of sync/async
>> requests together. I guess we wouldn't want that. The only check we can
>> skip in case of plug merge, is whether bio and rq beong to same task/cfqq
>> or not.
>>
>> May be separate elevator functions for plug merge (without lock) and
>> elevator merge (with lock) will do?
> 
> I don't think so.  As I wrote before, plugging is about issuing, not
> scheduling.  In a sense, it's a block layer service to upper layers to
> make building larger request easier such that block layer users can do
> "plug - issue multiple bios of whatever size - flush" instead of
> having to manually build series of maximum sized bios.  As such, bios
> submitted during a single plugging fare naturally expected to be for
> the same purpose.  They all belong to single IO unit after all.
> 
> Note that although request is involved in plug merging, the request
> isn't known to elevator (other than set_request(), that is).  The only
> reason we go half-way through request issuing, stash request, do plug
> merging and then do the rest on flush is because block layer doesn't
> have a way to deal with arbitrarily sized bios.  We're piggy backing
> on request merge logic because that's the only way we know how to
> group bios.
> 
> In the long run, I think plugging should be done with bios.  There's
> no reason to care about requests at that level.  We're just collecting
> bios issued as a unit and are likely to be close to each other.  If
> request_queue can deal with arbitrarily sized bios (and list of them),
> we can simply collect bios and flush down on unplug.  This would be
> much cleaner, make plugging useable for bio based drivers and allow
> simplifying stacking drivers.

In the longer term, plugging and merging should be much less needed.
Most file systems should have done this upfront already, so the need for
it should be much smaller. For the eventual multiqueue setup, I don't
plan on doing much work around merging at all. Perhaps some basic
merging for when it hits the final queue, but not at submit time.

-- 
Jens Axboe


  reply	other threads:[~2012-01-18 19:07 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-17 20:18 Kernel crash in icq_free_icq_rcu Vivek Goyal
2012-01-17 20:19 ` Jens Axboe
2012-01-17 20:40   ` Vivek Goyal
2012-01-17 20:42     ` Jens Axboe
2012-01-17 20:58       ` Vivek Goyal
2012-01-17 21:01         ` Vivek Goyal
2012-01-17 21:48 ` Tejun Heo
2012-01-17 22:07   ` Vivek Goyal
2012-01-18  1:01     ` Shaohua Li
2012-01-18  1:03       ` Tejun Heo
2012-01-18  1:05         ` Shaohua Li
2012-01-18  1:11           ` Tejun Heo
2012-01-18  1:30             ` Shaohua Li
2012-01-18  2:26               ` Shaohua Li
2012-01-18  4:23                 ` Shaohua Li
2012-01-18  6:03               ` Shaohua Li
2012-01-18 13:51                 ` Vivek Goyal
2012-01-18 14:20                   ` Vivek Goyal
2012-01-18 16:09                     ` Tejun Heo
2012-01-18 16:24                       ` Jens Axboe
2012-01-18 16:31                         ` Jens Axboe
2012-01-18 16:36                           ` Vivek Goyal
2012-01-18 17:10                             ` Tejun Heo
2012-01-18 19:07                               ` Jens Axboe [this message]
2012-01-18 19:05                             ` Jens Axboe
2012-01-18 16:55                           ` Tejun Heo
2012-01-18 16:07                 ` Tejun Heo
2012-01-19  1:41                   ` [patch]block: fix NULL icq_cache reference Shaohua Li
2012-01-19  1:43                     ` Tejun Heo
2012-01-19  8:20                     ` 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=4F171873.9040308@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=koverstreet@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=shaohua.li@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).