linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Axboe <jaxboe@fusionio.com>
To: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: device-mapper development <dm-devel@redhat.com>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	Promise_Linux <Promise_Linux@promise.com>
Subject: Re: block: Deprecate QUEUE_FLAG_CLUSTER and use queue_limits instead
Date: Thu, 25 Nov 2010 19:43:01 +0100	[thread overview]
Message-ID: <4CEEAE35.5060805@fusionio.com> (raw)
In-Reply-To: <yq1vd3lmkdt.fsf_-_@sermon.lab.mkp.net>

On 2010-11-25 17:35, Martin K. Petersen wrote:
> 
> When stacking devices a request_queue is not always available. This
> forced us to have a no_cluster flag in the queue_limits that could be
> used as a carrier until the request_queue had been set up for a
> metadevice.
> 
> There were several problems with that approach. First of all it was up
> to the stacking device to remember to set queue flag after stacking had
> completed. Also, the queue flag and the queue limits had to be kept in
> sync at all times. We got that wrong, which could lead to us issuing
> commands that went beyond the max scatterlist limit set by the driver.
> 
> The proper fix is to avoid having two flags for tracking the same thing.
> We deprecate QUEUE_FLAG_CLUSTER and use the queue limit directly in the
> block layer merging functions. The queue_limit 'no_cluster' is turned
> into 'cluster' to avoid double negatives and to ease stacking.
> Clustering defaults to being enabled as before. The queue flag logic is
> removed from the stacking function, and explicitly setting the cluster
> flag is no longer necessary in DM and MD.

Great, the two different values and needing to sync them was horrible.
What kind of testing did you do? Have to be a little extra careful at
this point.

> @@ -87,7 +87,7 @@ EXPORT_SYMBOL(blk_recount_segments);
>  static int blk_phys_contig_segment(struct request_queue *q, struct bio *bio,
>  				   struct bio *nxt)
>  {
> -	if (!test_bit(QUEUE_FLAG_CLUSTER, &q->queue_flags))
> +	if (blk_queue_cluster(q))
>  		return 0;

Oops, inverted this one.

-- 
Jens Axboe


  reply	other threads:[~2010-11-25 18:43 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-20  1:24 QUEUE_FLAG_CLUSTER? Ed Lin - PTU
2010-11-25 15:21 ` [dm-devel] QUEUE_FLAG_CLUSTER? Martin K. Petersen
2010-11-25 16:35   ` block: Deprecate QUEUE_FLAG_CLUSTER and use queue_limits instead Martin K. Petersen
2010-11-25 18:43     ` Jens Axboe [this message]
2010-11-26  0:37       ` Martin K. Petersen
2010-11-26  2:28         ` Mike Snitzer
2010-11-26  2:53           ` Martin K. Petersen
2010-11-26  9:00           ` Jens Axboe
2010-12-01 18:43         ` Jens Axboe
2010-11-29 19:08   ` [dm-devel] QUEUE_FLAG_CLUSTER? Ed Lin - PTU
2010-11-30 18:40     ` Martin K. Petersen
2010-12-07 21:48       ` [stable] " Greg KH

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=4CEEAE35.5060805@fusionio.com \
    --to=jaxboe@fusionio.com \
    --cc=Promise_Linux@promise.com \
    --cc=dm-devel@redhat.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.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).