Re: [RFC][PATCH v2]Add pm8001 SAS/SATA HBA driver

Rolf Eike Beer
2009-10-10T19:27:57Z

[-- Attachment #1: Type: Text/Plain, Size: 906 bytes --]

Grant Grundler wrote:

> Later in the patch I see:
> + pm8001_ha->tags = kmalloc(sizeof(unsigned long)*PM8001_MAX_DEVICES,
> + GFP_KERNEL);
>
> What are we limiting to 1024 devices? LUNS?
> I noticed the constant was also used in an initialization loop too.

There are some other things that I find suspicious at this point:

-for allocations of array kcalloc() is usually the way to go as it does
overflow checks for the multiplication result (not needed here with the
current values, but doesn't really hurt in init path either)
-using sizeof(*pm8001_ha->tags) would make sure it's the correct size no
matter what type this will ever be, because ...
-unsigned long has different size on 32 and 64 bit, I don't know if that makes
sense here. AFAICT this is sort of bitmap here so this probably doesn't need
to double it's size on 64 bit.

Greetings,

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]