drivers/scsi/aic7xxx/aic7xxx_osm.c: warning is error

drivers/scsi/aic7xxx/aic7xxx_osm.c: warning is error

[axel]

hi,

today compiled 2.5.69-bk8 with gcc version 3.3 20030510 and a warning in
drivers/scsi/aic7xxx/aic7xxx_osm.c resulted in an error because of gcc flag
-Werror.

  gcc -Wp,-MD,drivers/scsi/aic7xxx/.aic7xxx_osm.o.d -D__KERNEL__ -Iinclude
-Wall -Wstrict-prototypes -Wno-trigraphs -O2 -fno-strict-aliasing
-fno-common -pipe -mpreferred-stack-boundary=2 -march=i586
-Iinclude/asm-i386/mach-default -fomit-frame-pointer -nostdinc -iwithprefix
include  -Idrivers/scsi -Werror  -DKBUILD_BASENAME=aic7xxx_osm
-DKBUILD_MODNAME=aic7xxx -c -o drivers/scsi/aic7xxx/aic7xxx_osm.o
drivers/scsi/aic7xxx/aic7xxx_osm.c
drivers/scsi/aic7xxx/aic7xxx_osm.c: In function `ahc_linux_map_seg':
drivers/scsi/aic7xxx/aic7xxx_osm.c:767: warning: integer constant is too
large for "long" type
make[3]: *** [drivers/scsi/aic7xxx/aic7xxx_osm.o] Error 1


regards,
axel siebenwirth

Re: drivers/scsi/aic7xxx/aic7xxx_osm.c: warning is error

[William Lee Irwin III]

On Wed, May 14, 2003 at 02:40:09AM +0200, axel@pearbough.net wrote:
> today compiled 2.5.69-bk8 with gcc version 3.3 20030510 and a warning in
> drivers/scsi/aic7xxx/aic7xxx_osm.c resulted in an error because of gcc flag
> -Werror.
>   gcc -Wp,-MD,drivers/scsi/aic7xxx/.aic7xxx_osm.o.d -D__KERNEL__ -Iinclude
> -Wall -Wstrict-prototypes -Wno-trigraphs -O2 -fno-strict-aliasing
> -fno-common -pipe -mpreferred-stack-boundary=2 -march=i586
> -Iinclude/asm-i386/mach-default -fomit-frame-pointer -nostdinc -iwithprefix
> include  -Idrivers/scsi -Werror  -DKBUILD_BASENAME=aic7xxx_osm
> -DKBUILD_MODNAME=aic7xxx -c -o drivers/scsi/aic7xxx/aic7xxx_osm.o
> drivers/scsi/aic7xxx/aic7xxx_osm.c
> drivers/scsi/aic7xxx/aic7xxx_osm.c: In function `ahc_linux_map_seg':
> drivers/scsi/aic7xxx/aic7xxx_osm.c:767: warning: integer constant is too
> large for "long" type
> make[3]: *** [drivers/scsi/aic7xxx/aic7xxx_osm.o] Error 1

Could you send in your .config? I can't reproduce it here (gcc 3.2).

Thanks.


-- wli

Re: drivers/scsi/aic7xxx/aic7xxx_osm.c: warning is error

[axel]

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

Hi William!

On Tue, 13 May 2003, William Lee Irwin III wrote:

> On Wed, May 14, 2003 at 02:40:09AM +0200, axel@pearbough.net wrote:
> > today compiled 2.5.69-bk8 with gcc version 3.3 20030510 and a warning in
> > drivers/scsi/aic7xxx/aic7xxx_osm.c resulted in an error because of gcc flag
> > -Werror.
> >   gcc -Wp,-MD,drivers/scsi/aic7xxx/.aic7xxx_osm.o.d -D__KERNEL__ -Iinclude
> > -Wall -Wstrict-prototypes -Wno-trigraphs -O2 -fno-strict-aliasing
> > -fno-common -pipe -mpreferred-stack-boundary=2 -march=i586
> > -Iinclude/asm-i386/mach-default -fomit-frame-pointer -nostdinc -iwithprefix
> > include  -Idrivers/scsi -Werror  -DKBUILD_BASENAME=aic7xxx_osm
> > -DKBUILD_MODNAME=aic7xxx -c -o drivers/scsi/aic7xxx/aic7xxx_osm.o
> > drivers/scsi/aic7xxx/aic7xxx_osm.c
> > drivers/scsi/aic7xxx/aic7xxx_osm.c: In function `ahc_linux_map_seg':
> > drivers/scsi/aic7xxx/aic7xxx_osm.c:767: warning: integer constant is too
> > large for "long" type
> > make[3]: *** [drivers/scsi/aic7xxx/aic7xxx_osm.o] Error 1
> 
> Could you send in your .config? I can't reproduce it here (gcc 3.2).

Here you go...

Best regards,
Axel Siebenwirth

[-- Attachment #2: config-2.5.69-bk8.gz --]
[-- Type: application/x-gunzip, Size: 5060 bytes --]

Re: drivers/scsi/aic7xxx/aic7xxx_osm.c: warning is error

[William Lee Irwin III]

On Wed, May 14, 2003 at 02:40:09AM +0200, axel@pearbough.net wrote:
> today compiled 2.5.69-bk8 with gcc version 3.3 20030510 and a warning in
> drivers/scsi/aic7xxx/aic7xxx_osm.c resulted in an error because of gcc flag
> -Werror.

I can't reproduce this with gcc-3.2; does this do better?

I also removed some extremely fishy arithmetic in a check for crossing
4GB boundaries; I hope you don't mind.


-- wli


diff -prauN linux-2.5.69-bk8-1/drivers/scsi/aic7xxx/aic7xxx_osm.c linux-2.5.69-bk8-2/drivers/scsi/aic7xxx/aic7xxx_osm.c
--- linux-2.5.69-bk8-1/drivers/scsi/aic7xxx/aic7xxx_osm.c	2003-05-13 17:26:56.000000000 -0700
+++ linux-2.5.69-bk8-2/drivers/scsi/aic7xxx/aic7xxx_osm.c	2003-05-13 19:56:26.000000000 -0700
@@ -744,18 +744,20 @@ ahc_linux_map_seg(struct ahc_softc *ahc,
 		      "Increase AHC_NSEG\n");
 
 	consumed = 1;
-	sg->addr = ahc_htole32(addr & 0xFFFFFFFF);
+	sg->addr = ahc_htole32(addr & ~0U);
 	scb->platform_data->xfer_len += len;
-	if (sizeof(bus_addr_t) > 4
-	 && (ahc->flags & AHC_39BIT_ADDRESSING) != 0) {
+	if (sizeof(bus_addr_t) > 4 &&
+			(ahc->flags & AHC_39BIT_ADDRESSING) != 0) {
 		/*
-		 * Due to DAC restrictions, we can't
-		 * cross a 4GB boundary.
+		 * Due to DAC restrictions, we can't cross 4GB boundaries.
+		 * Right shift by 30 to find GB-granularity placement
+		 * without getting tripped up by anal compilers.
 		 */
-		if ((addr ^ (addr + len - 1)) & ~0xFFFFFFFF) {
+		if ((addr >> 30) < 4 && ((addr + len - 1) >> 30) >= 4) {
 			struct	 ahc_dma_seg *next_sg;
 			uint32_t next_len;
 
+			/* somebody clean this up to return an error */
 			printf("Crossed Seg\n");
 			if ((scb->sg_count + 2) > AHC_NSEG)
 				panic("Too few segs for dma mapping.  "
@@ -764,12 +766,22 @@ ahc_linux_map_seg(struct ahc_softc *ahc,
 			consumed++;
 			next_sg = sg + 1;
 			next_sg->addr = 0;
-			next_len = 0x100000000 - (addr & 0xFFFFFFFF);
+
+			/*
+			 * 2's complement arithmetic assumed.
+			 * We want: 4GB - low 32 bits of addr
+			 * to find the length of the low segment
+			 * and to subtract it out from the high
+			 */
+			next_len = -((uint32_t)addr);
 			len -= next_len;
-			next_len |= ((addr >> 8) + 0x1000000) & 0x7F000000;
+
+			/* c.f. struct ahc_dma_seg for meaning of high byte */
+			next_len |= ((addr >> 8) + AHC_SG_LEN_MASK + 1)
+						& AHC_SG_HIGH_ADDR_MASK;
 			next_sg->len = ahc_htole32(next_len);
 		}
-		len |= (addr >> 8) & 0x7F000000;
+		len |= (addr >> 8) & AHC_SG_HIGH_ADDR_MASK;
 	}
 	sg->len = ahc_htole32(len);
 	return (consumed);

Re: drivers/scsi/aic7xxx/aic7xxx_osm.c: warning is error

[William Lee Irwin III]

On Wed, May 14, 2003 at 02:40:09AM +0200, axel@pearbough.net wrote:
>> today compiled 2.5.69-bk8 with gcc version 3.3 20030510 and a warning in
>> drivers/scsi/aic7xxx/aic7xxx_osm.c resulted in an error because of gcc flag
>> -Werror.

On Tue, May 13, 2003 at 08:18:26PM -0700, William Lee Irwin III wrote:
> I can't reproduce this with gcc-3.2; does this do better?
> I also removed some extremely fishy arithmetic in a check for crossing
> 4GB boundaries; I hope you don't mind.

I installed gcc-3.3 and reproduced it and sent in a slightly different
fix.


-- wli

Re: drivers/scsi/aic7xxx/aic7xxx_osm.c: warning is error

[Justin T. Gibbs]

> I can't reproduce this with gcc-3.2; does this do better?
> 
> I also removed some extremely fishy arithmetic in a check for crossing
> 4GB boundaries; I hope you don't mind.

I mind.  The replacement code is still wrong.

>  
>  	consumed = 1;
> -	sg->addr = ahc_htole32(addr & 0xFFFFFFFF);
> +	sg->addr = ahc_htole32(addr & ~0U);

This assumes that an unsigned int is 32bits.  The old code assumed
that a long is at least 32bits.  Constants are promoted up to long
if they will not fit in an int.

>  	scb->platform_data->xfer_len += len;
> -	if (sizeof(bus_addr_t) > 4
> -	 && (ahc->flags & AHC_39BIT_ADDRESSING) != 0) {
> +	if (sizeof(bus_addr_t) > 4 &&
> +			(ahc->flags & AHC_39BIT_ADDRESSING) != 0) {

Superfluous style change.  The original style is intended and you will
see that this style is consistently used throughout the driver.

>  		/*
> -		 * Due to DAC restrictions, we can't
> -		 * cross a 4GB boundary.
> +		 * Due to DAC restrictions, we can't cross 4GB boundaries.
> +		 * Right shift by 30 to find GB-granularity placement
> +		 * without getting tripped up by anal compilers.
>  		 */
> -		if ((addr ^ (addr + len - 1)) & ~0xFFFFFFFF) {
> +		if ((addr >> 30) < 4 && ((addr + len - 1) >> 30) >= 4) {

What happens if the starting address is 0x00000070XXXXXXXX?  We cannot
cross any 4GB boundary in the entire 64bit address space.  The previous
code did that with the exception that the constant must be promoted
to ULL:

		if ((addr ^ (addr + len - 1)) & 0xFFFFFFFF00000000ULL) {

In other words, the high 32bits of the starting and ending address had 
better be the same (x ^ x == 0).

> @@ -764,12 +766,22 @@ ahc_linux_map_seg(struct ahc_softc *ahc,
>  			consumed++;
>  			next_sg = sg + 1;
>  			next_sg->addr = 0;
> -			next_len = 0x100000000 - (addr & 0xFFFFFFFF);
> +
> +			/*
> +			 * 2's complement arithmetic assumed.
> +			 * We want: 4GB - low 32 bits of addr
> +			 * to find the length of the low segment
> +			 * and to subtract it out from the high
> +			 */
> +			next_len = -((uint32_t)addr);
>  			len -= next_len;

This still leaves a bug. next_len and len are reversed.  I also feel
that the previous code, if properly promoted, is clearer.  There is no
need for a comment and the compiler should do the same truncation as
is performed in the above code.

> -			next_len |= ((addr >> 8) + 0x1000000) & 0x7F000000;
> +
> +			/* c.f. struct ahc_dma_seg for meaning of high byte */
> +			next_len |= ((addr >> 8) + AHC_SG_LEN_MASK + 1)
> +						& AHC_SG_HIGH_ADDR_MASK;

Using (AHC_SG_LEN_MASK + 1) to mean 4GB >> 8 is not a way to make the
code more readable.

My patch for these issues follows.  A more formal BK submission will
follow tomorrow.

Comments have indicated since the 2.4.X days that Linux will never allocate
segments that cross a 4GB boundary.  If this is truely enforced, then this
code can just be removed.  It was only added out of paranoia (hence the
printf) while adding high address support to the driver.

--
Justin

==== //depot/aic7xxx/linux/drivers/scsi/aic7xxx/aic7xxx_osm.c#222 (text+ko) -
//depot/aic7xxx/linux/drivers/scsi/aic7xxx/aic7xxx_osm.c#224 (text+ko) ==== content
@@ -751,12 +751,14 @@
 	scb->platform_data->xfer_len += len;
 	if (sizeof(bus_addr_t) > 4
 	 && (ahc->flags & AHC_39BIT_ADDRESSING) != 0) {
+
 		/*
 		 * Due to DAC restrictions, we can't
 		 * cross a 4GB boundary.
 		 */
-		if ((addr ^ (addr + len - 1)) & ~0xFFFFFFFF) {
+		if ((addr ^ (addr + len - 1)) & 0xFFFFFFFF00000000ULL) {
 			struct	 ahc_dma_seg *next_sg;
+			uint32_t first_len;
 			uint32_t next_len;
 
 			printf("Crossed Seg\n");
@@ -767,12 +769,14 @@
 			consumed++;
 			next_sg = sg + 1;
 			next_sg->addr = 0;
-			next_len = 0x100000000 - (addr & 0xFFFFFFFF);
-			len -= next_len;
-			next_len |= ((addr >> 8) + 0x1000000) & 0x7F000000;
+			first_len = 0x100000000ULL - (addr & 0xFFFFFFFF);
+			next_len = len - first_len;
+			len = first_len;
+			next_len |=
+			    ((addr >> 8) + 0x1000000) & AHC_SG_HIGH_ADDR_MASK;
 			next_sg->len = ahc_htole32(next_len);
 		}
-		len |= (addr >> 8) & 0x7F000000;
+		len |= (addr >> 8) & AHC_SG_HIGH_ADDR_MASK;
 	}
 	sg->len = ahc_htole32(len);
 	return (consumed);
==== //depot/aic7xxx/linux/drivers/scsi/aic7xxx/aic79xx_osm.c#161 (text+ko) -
//depot/aic7xxx/linux/drivers/scsi/aic7xxx/aic79xx_osm.c#163 (text+ko) ==== content
@@ -761,8 +761,9 @@
 		 * Due to DAC restrictions, we can't
 		 * cross a 4GB boundary.
 		 */
-		if ((addr ^ (addr + len - 1)) & ~0xFFFFFFFF) {
+		if ((addr ^ (addr + len - 1)) & 0xFFFFFFFF00000000ULL) {
 			struct	 ahd_dma_seg *next_sg;
+			uint32_t first_len;
 			uint32_t next_len;
 
 			printf("Crossed Seg\n");
@@ -773,12 +774,14 @@
 			consumed++;
 			next_sg = sg + 1;
 			next_sg->addr = 0;
-			next_len = 0x100000000 - (addr & 0xFFFFFFFF);
-			len -= next_len;
-			next_len |= ((addr >> 8) + 0x1000000) & 0x7F000000;
+			first_len = 0x100000000ULL - (addr & 0xFFFFFFFF);
+			next_len = len - first_len;
+			len = next_len;
+			next_len |=
+			    ((addr >> 8) + 0x1000000) & AHD_SG_HIGH_ADDR_MASK;
 			next_sg->len = ahd_htole32(next_len);
 		}
-		len |= (addr >> 8) & 0x7F000000;
+		len |= (addr >> 8) & AHD_SG_HIGH_ADDR_MASK;
 	}
 	sg->len = ahd_htole32(len);
 	return (consumed);

Re: drivers/scsi/aic7xxx/aic7xxx_osm.c: warning is error

[William Lee Irwin III]

On Wed, May 14, 2003 at 12:05:04AM -0600, Justin T. Gibbs wrote:
> I mind.  The replacement code is still wrong.

Let's see.


>>  	consumed = 1;
>> -	sg->addr = ahc_htole32(addr & 0xFFFFFFFF);
>> +	sg->addr = ahc_htole32(addr & ~0U);

On Wed, May 14, 2003 at 12:05:04AM -0600, Justin T. Gibbs wrote:
> This assumes that an unsigned int is 32bits.  The old code assumed
> that a long is at least 32bits.  Constants are promoted up to long
> if they will not fit in an int.

gcc never uses that model; hence it's fine for Linux. unsigned int is
32-bit on 64-bit and 32-bit, and it's actually guaranteed enough to
trip up others creating constant initializer bitmasks like
task->cpus_allowed and for other parts of the kernel to rely on it
for correctness. ILP64 is not supported by Linux.


On Wed, May 14, 2003 at 12:05:04AM -0600, Justin T. Gibbs wrote:
>>  	scb->platform_data->xfer_len += len;
>> -	if (sizeof(bus_addr_t) > 4
>> -	 && (ahc->flags & AHC_39BIT_ADDRESSING) != 0) {
>> +	if (sizeof(bus_addr_t) > 4 &&
>> +			(ahc->flags & AHC_39BIT_ADDRESSING) != 0) {

On Wed, May 14, 2003 at 12:05:04AM -0600, Justin T. Gibbs wrote:
> Superfluous style change.  The original style is intended and you will
> see that this style is consistently used throughout the driver.

Linux style conformance would be better.


> >  		/*
> > -		 * Due to DAC restrictions, we can't
> > -		 * cross a 4GB boundary.
> > +		 * Due to DAC restrictions, we can't cross 4GB boundaries.
> > +		 * Right shift by 30 to find GB-granularity placement
> > +		 * without getting tripped up by anal compilers.
> >  		 */
> > -		if ((addr ^ (addr + len - 1)) & ~0xFFFFFFFF) {
> > +		if ((addr >> 30) < 4 && ((addr + len - 1) >> 30) >= 4) {

On Wed, May 14, 2003 at 12:05:04AM -0600, Justin T. Gibbs wrote:
> What happens if the starting address is 0x00000070XXXXXXXX?  We cannot
> cross any 4GB boundary in the entire 64bit address space.  The previous
> code did that with the exception that the constant must be promoted
> to ULL:
> 		if ((addr ^ (addr + len - 1)) & 0xFFFFFFFF00000000ULL) {
> In other words, the high 32bits of the starting and ending address had 
> better be the same (x ^ x == 0).

The constant is never promoted to ULL by gcc. I've demonstrated that in
another post.


> > @@ -764,12 +766,22 @@ ahc_linux_map_seg(struct ahc_softc *ahc,
> >  			consumed++;
> >  			next_sg = sg + 1;
> >  			next_sg->addr = 0;
> > -			next_len = 0x100000000 - (addr & 0xFFFFFFFF);
> > +
> > +			/*
> > +			 * 2's complement arithmetic assumed.
> > +			 * We want: 4GB - low 32 bits of addr
> > +			 * to find the length of the low segment
> > +			 * and to subtract it out from the high
> > +			 */
> > +			next_len = -((uint32_t)addr);
> >  			len -= next_len;

On Wed, May 14, 2003 at 12:05:04AM -0600, Justin T. Gibbs wrote:
> This still leaves a bug. next_len and len are reversed.  I also feel
> that the previous code, if properly promoted, is clearer.  There is no
> need for a comment and the compiler should do the same truncation as
> is performed in the above code.

It is never promoted. There is also no difference between what
changed it to and what it did before besides the compile error.


>> -			next_len |= ((addr >> 8) + 0x1000000) & 0x7F000000;
>> +
>> +			/* c.f. struct ahc_dma_seg for meaning of high byte */
>> +			next_len |= ((addr >> 8) + AHC_SG_LEN_MASK + 1)
>> +						& AHC_SG_HIGH_ADDR_MASK;

On Wed, May 14, 2003 at 12:05:04AM -0600, Justin T. Gibbs wrote:
> Using (AHC_SG_LEN_MASK + 1) to mean 4GB >> 8 is not a way to make the
> code more readable.
> My patch for these issues follows.  A more formal BK submission will
> follow tomorrow.

I hadn't the foggiest idea you had that in mind. The best I could
reverse-engineer it to was the above.


On Wed, May 14, 2003 at 12:05:04AM -0600, Justin T. Gibbs wrote:
> Comments have indicated since the 2.4.X days that Linux will never allocate
> segments that cross a 4GB boundary.  If this is truely enforced, then this
> code can just be removed.  It was only added out of paranoia (hence the
> printf) while adding high address support to the driver.

I've heard the same from others.

All the above defense of my patch aside I don't have any issues with
your patch to resolve the compile errors and am fine with your
including it instead of my own.


-- wli

Re: drivers/scsi/aic7xxx/aic7xxx_osm.c: warning is error

[Mike Anderson]

Justin T. Gibbs [gibbs@scsiguy.com] wrote:
> Comments have indicated since the 2.4.X days that Linux will never allocate
> segments that cross a 4GB boundary.  If this is truely enforced, then this
> code can just be removed.  It was only added out of paranoia (hence the
> printf) while adding high address support to the driver.

Jens can give the more complete answer on enforcement, and also correct
any mis-statements I made.

Base on the queue values below the aic7xxx driver should see the
following characteristics on IO. The IO should be for no more than 8k
made up of no more than 128 sg entries with no segment crossing the
seg_boundary_mask.

Adaptec AIC7xxx driver version: 6.2.33
scsi_alloc_queue: queue for aic7xxx
bounce_pfn: 0xfffff
bounce_gfp: 0x10 (GFP_NOIO)
queue_flags: 0x1 (QUEUE_FLAG_QUEUED)
max_sectors: 0x2000 (8192)
max_phys_segments: 0x80 (128)
max_hw_segments: 0x80 (128)
hardsect_size: 0x200 (512)
max_segment_size: 0x10000 (65536)
seg_boundary_mask: 0xffffffff
dma_alignment: 0x1ff (511)

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: drivers/scsi/aic7xxx/aic7xxx_osm.c: warning is error

[Jens Axboe]

On Wed, May 14 2003, Mike Anderson wrote:
> Justin T. Gibbs [gibbs@scsiguy.com] wrote:
> > Comments have indicated since the 2.4.X days that Linux will never allocate
> > segments that cross a 4GB boundary.  If this is truely enforced, then this
> > code can just be removed.  It was only added out of paranoia (hence the
> > printf) while adding high address support to the driver.
> 
> Jens can give the more complete answer on enforcement, and also correct
> any mis-statements I made.

This property can be toggled with blk_queue_segment_boundary, and we do
default to setting a 4GB boundary mask. So you can be sure that a
request will never straddle a 4GB boundary.

> Base on the queue values below the aic7xxx driver should see the
> following characteristics on IO. The IO should be for no more than 8k
> made up of no more than 128 sg entries with no segment crossing the
> seg_boundary_mask.

I suppose you mean for no more than 8k sectors, ie 4MiB of data.

> Adaptec AIC7xxx driver version: 6.2.33
> scsi_alloc_queue: queue for aic7xxx
> bounce_pfn: 0xfffff
> bounce_gfp: 0x10 (GFP_NOIO)
> queue_flags: 0x1 (QUEUE_FLAG_QUEUED)
> max_sectors: 0x2000 (8192)
> max_phys_segments: 0x80 (128)
> max_hw_segments: 0x80 (128)
> hardsect_size: 0x200 (512)
> max_segment_size: 0x10000 (65536)
> seg_boundary_mask: 0xffffffff

that is the key here.

> dma_alignment: 0x1ff (511)

So to recap, aic7xxx will never see a request that exceeds one of the
above values. Total request size will always be equal to or below 4MiB,
less than or equal to 128 segments, and will never cross a 4GB memory
boundary. Memory above pfn 0xfffff (4GB) will be bounced, but this could
be because that's just the amount of memory the box has you dumped this
info from.

-- 
Jens Axboe

Re: drivers/scsi/aic7xxx/aic7xxx_osm.c: warning is error

[William Lee Irwin III]

On Wed, May 14 2003, Mike Anderson wrote:
[...]
>> bounce_pfn: 0xfffff
[...]

That might explain the performance problem reported to me not long
after I posted the thing.

On Wed, May 14, 2003 at 09:54:07AM +0200, Jens Axboe wrote:
> So to recap, aic7xxx will never see a request that exceeds one of the
> above values. Total request size will always be equal to or below 4MiB,
> less than or equal to 128 segments, and will never cross a 4GB memory
> boundary. Memory above pfn 0xfffff (4GB) will be bounced, but this could
> be because that's just the amount of memory the box has you dumped this
> info from.

Hey, I got a bug report of a compile error and dove into bogus-looking
code around it meant to cater to antediluvian kernel versions and the
maintainer's preference for an identical driver across bunches of
kernel versions. I've heard the bit about the midlayer preventing these
conditions entirely a dozen times in the past 2 hours. =(

-- wli

Re: drivers/scsi/aic7xxx/aic7xxx_osm.c: warning is error

[Mike Anderson]

Jens Axboe [axboe@suse.de] wrote:
> On Wed, May 14 2003, Mike Anderson wrote:
> > Justin T. Gibbs [gibbs@scsiguy.com] wrote:
> > > Comments have indicated since the 2.4.X days that Linux will never allocate
> > > segments that cross a 4GB boundary.  If this is truely enforced, then this
> > > code can just be removed.  It was only added out of paranoia (hence the
> > > printf) while adding high address support to the driver.
> > 
> > Jens can give the more complete answer on enforcement, and also correct
> > any mis-statements I made.
> 
> This property can be toggled with blk_queue_segment_boundary, and we do
> default to setting a 4GB boundary mask. So you can be sure that a
> request will never straddle a 4GB boundary.
> 
> > Base on the queue values below the aic7xxx driver should see the
> > following characteristics on IO. The IO should be for no more than 8k
> > made up of no more than 128 sg entries with no segment crossing the
> > seg_boundary_mask.
> 
> I suppose you mean for no more than 8k sectors, ie 4MiB of data.
> 

Yes, late night typing (my brain went to sleep before my fingers).

> > Adaptec AIC7xxx driver version: 6.2.33
> > scsi_alloc_queue: queue for aic7xxx
> > bounce_pfn: 0xfffff
> > bounce_gfp: 0x10 (GFP_NOIO)
> > queue_flags: 0x1 (QUEUE_FLAG_QUEUED)
> > max_sectors: 0x2000 (8192)
> > max_phys_segments: 0x80 (128)
> > max_hw_segments: 0x80 (128)
> > hardsect_size: 0x200 (512)
> > max_segment_size: 0x10000 (65536)
> > seg_boundary_mask: 0xffffffff
> 
> that is the key here.
> 
> > dma_alignment: 0x1ff (511)
> 
> So to recap, aic7xxx will never see a request that exceeds one of the
> above values. Total request size will always be equal to or below 4MiB,
> less than or equal to 128 segments, and will never cross a 4GB memory
> boundary. Memory above pfn 0xfffff (4GB) will be bounced, but this could
> be because that's just the amount of memory the box has you dumped this
> info from.
> 

The system I dumped the info from only has 256MB. SCSI calls
blk_queue_bounce_limit with the return value of
scsi_calculate_bounce_limit which takes the dma_mask set by the adapter.
Some adapters set this value based the adapters capabilities and
CONFIG_HIGHMEM64G, but the aic7xxx also checks the memsize.

My config on this system also has (CONFIG_HIGHMEM64G, CONFIG_HIGHMEM,
and CONFIG_X86_PAE) set.

-andmike
--
Michael Anderson
andmike@us.ibm.com