[BLOCK] phys_contig implies hw_contig

[BLOCK] phys_contig implies hw_contig

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 477 bytes --]

Hi:

In ll_merge_requests_fn, it is checking blk_hw_contig_segments even if
blk_phys_contig_segments succeeds.  This means that it may cause two
physically contiguous segments to be separated because the hw check
fails.

This patch fixes that.

Cheers,
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 1130 bytes --]

Index: kernel-source-2.5/drivers/block/ll_rw_blk.c
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/drivers/block/ll_rw_blk.c,v
retrieving revision 1.9
diff -u -r1.9 ll_rw_blk.c
--- kernel-source-2.5/drivers/block/ll_rw_blk.c	11 Oct 2003 06:29:20 -0000	1.9
+++ kernel-source-2.5/drivers/block/ll_rw_blk.c	1 Nov 2003 02:24:52 -0000
@@ -1046,16 +1046,16 @@
 		return 0;
 
 	total_phys_segments = req->nr_phys_segments + next->nr_phys_segments;
-	if (blk_phys_contig_segment(q, req->biotail, next->bio))
+	total_hw_segments = req->nr_hw_segments + next->nr_hw_segments;
+
+	if (blk_phys_contig_segment(q, req->biotail, next->bio)) {
 		total_phys_segments--;
+		total_hw_segments--;
+	} else if (blk_hw_contig_segment(q, req->biotail, next->bio))
+		total_hw_segments--;
 
 	if (total_phys_segments > q->max_phys_segments)
 		return 0;
-
-	total_hw_segments = req->nr_hw_segments + next->nr_hw_segments;
-	if (blk_hw_contig_segment(q, req->biotail, next->bio))
-		total_hw_segments--;
-
 	if (total_hw_segments > q->max_hw_segments)
 		return 0;
 

Re: [BLOCK] phys_contig implies hw_contig

[David S. Miller]

On Sat, 1 Nov 2003 13:31:27 +1100
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> In ll_merge_requests_fn, it is checking blk_hw_contig_segments even if
> blk_phys_contig_segments succeeds.  This means that it may cause two
> physically contiguous segments to be separated because the hw check
> fails.

Your analysis assumes that phys contig implies hw contig, I
believe it does not.  There may be limitations in the VMERGE
mechanism a platform implements that causes this situation to
arise.

Re: [BLOCK] phys_contig implies hw_contig

[Herbert Xu]

On Sat, Nov 01, 2003 at 08:45:47PM -0800, David S. Miller wrote:
> On Sat, 1 Nov 2003 13:31:27 +1100
> Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
> > In ll_merge_requests_fn, it is checking blk_hw_contig_segments even if
> > blk_phys_contig_segments succeeds.  This means that it may cause two
> > physically contiguous segments to be separated because the hw check
> > fails.
> 
> Your analysis assumes that phys contig implies hw contig, I
> believe it does not.  There may be limitations in the VMERGE
> mechanism a platform implements that causes this situation to
> arise.

OK, if that's the case, then we better change blk_recount_segments
as it assumes this.

Cheers,
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [BLOCK] phys_contig implies hw_contig

[Herbert Xu]

On Sun, Nov 02, 2003 at 03:47:34PM +1100, herbert wrote:
> On Sat, Nov 01, 2003 at 08:45:47PM -0800, David S. Miller wrote:
>
> > Your analysis assumes that phys contig implies hw contig, I
> > believe it does not.  There may be limitations in the VMERGE
> > mechanism a platform implements that causes this situation to
> > arise.
> 
> OK, if that's the case, then we better change blk_recount_segments
> as it assumes this.

Bug David that can't be right.  If two segments are physically contiguous,
then they don't need to be remapped to be virtually continguous, right?
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [BLOCK] phys_contig implies hw_contig

[David S. Miller]

On Mon, 3 Nov 2003 09:10:26 +1100
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Sun, Nov 02, 2003 at 03:47:34PM +1100, herbert wrote:
> > On Sat, Nov 01, 2003 at 08:45:47PM -0800, David S. Miller wrote:
> >
> > > Your analysis assumes that phys contig implies hw contig, I
> > > believe it does not.  There may be limitations in the VMERGE
> > > mechanism a platform implements that causes this situation to
> > > arise.
> > 
> > OK, if that's the case, then we better change blk_recount_segments
> > as it assumes this.
> 
> Bug David that can't be right.  If two segments are physically contiguous,
> then they don't need to be remapped to be virtually continguous, right?

Yes it sounds silly.

To be honest I'm more concerned that things are consistent.
Therefore I recommend that your original patch is applied.