public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <jens.axboe@oracle.com>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Nikanth Karthikesan <knikanth@suse.de>,
	linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org,
	FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: Re: [PATCH] BUG: nr_phys_segments cannot be less than nr_hw_segments
Date: Thu, 2 Oct 2008 19:13:57 +0200	[thread overview]
Message-ID: <20081002171356.GO19428@kernel.dk> (raw)
In-Reply-To: <1222967563.3222.30.camel@localhost.localdomain>

On Thu, Oct 02 2008, James Bottomley wrote:
> On Thu, 2008-10-02 at 18:58 +0200, Jens Axboe wrote:
> > On Thu, Oct 02 2008, James Bottomley wrote:
> > > The bug would appear to be that we sometimes only look at q->max_sectors
> > > when deciding on mergability.  Either we have to insist on max_sectors
> > > <= hw_max_sectors, or we have to start using min(q->max_sectors,
> > > q->max_hw_sectors) for this.
> > 
> > q->max_sectors MUST always be <= q->max_hw_sectors, otherwise we could
> > be sending down requests that are too large for the device to handle. So
> > that condition would be a big bug. The sysfs interface checks for this,
> > and blk_queue_max_sectors() makes sure that is true as well.
> 
> Yes, that seems always to be enforced.  Perhaps there are other ways of
> tripping this problem ... I'm still sure if it occurs it's because we do
> a physical merge where a virtual merge is forbidden.
> 
> > The fixes proposed still look weird. There is no phys vs hw segment
> > constraints, the request must adhere to the limits set by both. It's
> > mostly a moot point anyway, as 2.6.28 will get rid of the hw accounting
> > anyway.
> 
> Agree all three proposed fixes look wrong.  However, if it's what I
> think, just getting rid of hw accounting won't fix the problem because
> we'll still have the case where a physical merge is forbidden by iommu
> constraints ... this still needs to be accounted for.
> 
> What we really need is a demonstration of what actually is going
> wrong ...

Yep, IIRC we both asked for that the last time as well... Nikanth?

-- 
Jens Axboe


  reply	other threads:[~2008-10-02 17:14 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-02 14:29 [PATCH] BUG: nr_phys_segments cannot be less than nr_hw_segments Nikanth Karthikesan
2008-10-02 15:03 ` James Bottomley
2008-10-02 16:58   ` Jens Axboe
2008-10-02 17:12     ` James Bottomley
2008-10-02 17:13       ` Jens Axboe [this message]
2008-10-03  5:28         ` Nikanth Karthikesan
2008-10-06 17:24         ` FUJITA Tomonori
2008-10-10 12:03           ` Jens Axboe
2008-10-10 12:32             ` FUJITA Tomonori
2008-10-10 12:37               ` Jens Axboe
2008-10-10 12:49                 ` FUJITA Tomonori
2008-10-02 15:05 ` Nikanth Karthikesan

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=20081002171356.GO19428@kernel.dk \
    --to=jens.axboe@oracle.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=knikanth@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    /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