On Wed, Nov 19, 2003 at 11:44:42AM -0800, James Jones wrote:
 > >
 > >
 > >       hammers[i++] = loop_dev;
 > >       nr_garts = i;
 > >#ifdef CONFIG_SMP
 > >       if (i == MAX_HAMMER_GARTS) {
 > >           printk(KERN_INFO PFX "Too many northbridges for AGP\n");
 > >           return -1;
 > >       }
 > >
 > 
 > Seems wrong to me... wouldn't this return -1 if say, MAX_HAMMER_GARTS == 
 > 1 and 1 gart was found ( nr_garts == i == 1 when the comparison is made 
 > ).  It would need to be:

MAX_HAMMER_GARTS can only be 1 if CONFIG_SMP=n, so the above code
is skipped, and we fall through, and return 0.

 > This would also be wrong, as the test would be too late, and hammer[] 
 > would be overflowed by the time the test is performed.  This is why the 
 > test was moved before the assignment in our patches.  The way we did it 
 > would handle the SMP and non-smp cases I believe, the code you quoted 
 > would only work right in the uniproc case.

That's how it should be.

If we have a UP system, with UP kernel, we just return 0 after
finding the first GART

If we have a UP system with an SMP kernel, we find the first GART,
and eventually end up returning 0 after finding no further garts.

If we have an SMP system with UP kernel, we exit early after
finding the first GART. The 2nd (And above) GARTs are irrelevant
when not running in SMP.

If we have an SMP system with an SMP kernel, we add however many
GARTs to the table, up to a limit of MAX_HAMMER_GARTS.

I don't see any problems.

                Dave



-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
Does SourceForge.net help you be more productive?  Does it
help you create better code?  SHARE THE LOVE, and help us help
YOU!  Click Here: http://sourceforge.net/donate/
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to