Patch?: linux-2.5.2-pre9/drivers/block/ll_rw_blk.c blk_rq_map_sg simplification

Patch?: linux-2.5.2-pre9/drivers/block/ll_rw_blk.c blk_rq_map_sg simplification

[Adam J. Richter]

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

Hello Jens,

	The following patch removes gotos from blk_rq_map_sg, making
it more readable and five lines shorter.  I think the compiler should
generate the same code.  I have not tested this other than to
verify that it compiles.

	If it works and looks good to you, I would appreciate it if
you would forward it to Linus for integration.  (I am also cc'ing this to
linux-kernel in case anyone spots a mistake on my part.)

	Also, if there is some other way by which you would like
me to submit future bio patches (if any), please let me know.

-- 
Adam J. Richter     __     ______________   4880 Stevens Creek Blvd, Suite 104
adam@yggdrasil.com     \ /                  San Jose, California 95129-1034
+1 408 261-6630         | g g d r a s i l   United States of America
fax +1 408 261-6631      "Free Software For The Rest Of Us."

[-- Attachment #2: ll.diff --]
[-- Type: text/plain, Size: 870 bytes --]

--- linux-2.5.2-pre8/drivers/block/ll_rw_blk.c	Sat Jan  5 15:33:53 2002
+++ linux/drivers/block/ll_rw_blk.c	Mon Jan  7 00:28:17 2002
@@ -474,18 +474,13 @@
 		bio_for_each_segment(bvec, bio, i) {
 			int nbytes = bvec->bv_len;
 
-			if (bvprv && cluster) {
-				if (sg[nsegs - 1].length + nbytes > q->max_segment_size)
-					goto new_segment;
-
-				if (!BIOVEC_PHYS_MERGEABLE(bvprv, bvec))
-					goto new_segment;
-				if (!BIOVEC_SEG_BOUNDARY(q, bvprv, bvec))
-					goto new_segment;
-
+			if (bvprv && cluster &&
+			    (sg[nsegs - 1].length + nbytes <=
+			     q->max_segment_size) &&
+			    BIOVEC_PHYS_MERGEABLE(bvprv, bvec) &&
+			    BIOVEC_SEG_BOUNDARY(q, bvprv, bvec)) {
 				sg[nsegs - 1].length += nbytes;
 			} else {
-new_segment:
 				memset(&sg[nsegs],0,sizeof(struct scatterlist));
 				sg[nsegs].page = bvec->bv_page;
 				sg[nsegs].length = nbytes;

Re: Patch?: linux-2.5.2-pre9/drivers/block/ll_rw_blk.c blk_rq_map_sg simplification

[Jens Axboe]

On Mon, Jan 07 2002, Adam J. Richter wrote:
> Hello Jens,
> 
> 	The following patch removes gotos from blk_rq_map_sg, making
> it more readable and five lines shorter.  I think the compiler should
> generate the same code.  I have not tested this other than to
> verify that it compiles.

Well, I really think the original is much more readable than the changed
version :-)

> 	Also, if there is some other way by which you would like
> me to submit future bio patches (if any), please let me know.

This is quite fine.

-- 
Jens Axboe

Re: Patch?: linux-2.5.2-pre9/drivers/block/ll_rw_blk.c blk_rq_map_sg simplification

[Dave Jones]

On Mon, 7 Jan 2002, Jens Axboe wrote:

> > 	The following patch removes gotos from blk_rq_map_sg, making
> > it more readable and five lines shorter.  I think the compiler should
> > generate the same code.  I have not tested this other than to
> > verify that it compiles.
> Well, I really think the original is much more readable than the changed
> version :-)

I agree. Upon seeing the patch, I was reminded of the monster
enormous conditional at http://gcc.gnu.org/projects/beginner.html
not quite _that_ bad, but getting there 8)

-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

Re: Patch?: linux-2.5.2-pre9/drivers/block/ll_rw_blk.c blk_rq_map_sg simplification

[Adam J. Richter]

>>> = Adam Richter (adam@yggdrasil.com)
>>  = Jens Axboe (axboe@suse.de)
>   = Dave Jones (davej@suse.de)
>>> 	The following patch removes gotos from blk_rq_map_sg, making
>>> it more readable and five lines shorter.  I think the compiler should
>>> generate the same code.  I have not tested this other than to
>>> verify that it compiles.
>> Well, I really think the original is much more readable than the changed
>> version :-)

>I agree. Upon seeing the patch, I was reminded of the monster
>enormous conditional at http://gcc.gnu.org/projects/beginner.html
>not quite _that_ bad, but getting there 8)

	The conditional is one level deep (it's just a list of
"and" conjunctions) and has a single conceptual meaning, "are
the segments joinable?"  For what it's worth, I really could not
understand the old version until I rewrote it without gotos.

Adam J. Richter     __     ______________   4880 Stevens Creek Blvd, Suite 104
adam@yggdrasil.com     \ /                  San Jose, California 95129-1034
+1 408 261-6630         | g g d r a s i l   United States of America
fax +1 408 261-6631      "Free Software For The Rest Of Us."

Re: Patch?: linux-2.5.2-pre9/drivers/block/ll_rw_blk.c blk_rq_map_sg simplification

[Dave Jones]

On Mon, 7 Jan 2002, Adam J. Richter wrote:

> 	The conditional is one level deep (it's just a list of
> "and" conjunctions) and has a single conceptual meaning, "are
> the segments joinable?"  For what it's worth, I really could not
> understand the old version until I rewrote it without gotos.

whitespace == a good thing.

-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs