>"A month of sundays ago Ilpo Ruotsalainen wrote:"
>>
>> While looking at the RAID1 code in 2.2.13ac3, I noticed the following
>> strangeness:
>>
>> static void * raid1_kmalloc (int size)
>> {
>> void * ptr;
>>
>> while (!(ptr = kmalloc (sizeof (raid1_conf_t), GFP_KERNEL)))
>> printk ("raid1: out of memory, retrying...\n");
>>
>> memset(ptr, 0, size);
>> return ptr;
>> }
>>
>> Why do we allocate sizeof(raid1_conf_t) bytes but clear size bytes? From
>
>The memset looks very wrong. Put a test in for it being oversize.
Still it looks more like the kmalloc is wrong, since this function is
called like this:
struct raid1_bh * r1_bh;
...
r1_bh = raid1_kmalloc (sizeof (struct raid1_bh));
...which should allocate sizeof(struct raid1_bh) bytes (and clear them),
not allocate sizeof(raid1_conf_t) bytes and clear sizeof(struct raid1_bh)
bytes...
>> the way this is called it seems like we should allocate size bytes too...
>> Also the busyloop here isn't nice, is it safe to reschedule here or
>
>Yes, kmalloc can sleep and it's inevitable. This is canonical code.
>
>> something if the memory allocation fails? (I don't like the n^42 messages
>
>It should be
>
> while (!(ptr = kmalloc (sizeof (raid1_conf_t), GFP_KERNEL))) {
> static int count = 0;
> if (count++ < 5)
> printk ("raid1: out of memory, retrying...\n");
> }
>
>or some such variant.
I'd still like to have reschedule or something in there, it loops VERY fast
as it is at least on UP machines when the kmalloc fails...
Perhaps print just one message for one call to raid1_kmalloc() no matter
how many times the kmalloc() in there actually fails would be right way to
print the message.
-- Ilpo Ruotsalainen - <lonewolf@iki.fi> - http://www.iki.fi/lonewolf/
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/