>But the kmalloc(... GFP_KERNEL); should never fail. It might hang forever
>waiting for memory, but it won't fail.
>The check is here is only to prevent disaster should the semantics of
>GFP_KERNEL change.
In practice this is almost always true ;) (since some task is going to be
killed before eating also the last free page of memory). But if all the
memory requests cames from the kernel (so using GFP_KERNEL), then - when
the free-area pool will be empty - kmalloc will fail without hanging
forever (this in 2.2.x and 2.3.x at least, I don't know 2.0.x).
Just to point out that nothing prevents GFP_KERNEL from failing. I
reproduced a complete OOM (0 pages free) also in pratice (well I admit, I
reproduced it by mistake with some bug, but at least it's been nice that
the other part of the kernel was still fine ;).
The only difference between GFP_KERNEL and GFP_ATOMIC is that GFP_KERNEL
will also try to generate some free page if the memory is low.
GFP_USER instead will fail if the system is not been able to generate some
free page while the memory was just low. (GFP_USER will fail even if there
are still some pages in the free-area pool)
BTW, while pressing CTRL+S in emacs searching for kmalloc I noticed
another problem of the same kind (almost harmless if the is still one DMA
available at insmod time). line 447:
tp = kmalloc(sizeof(*tp), GFP_KERNEL | GFP_DMA);
memset(tp, 0, sizeof(*tp));
BTW, I can't see why the private struct must be stored in a DMA page.
I also noticed another little problem:
tp->tx_bufs = kmalloc(TX_BUF_SIZE * NUM_TX_DESC, GFP_KERNEL);
tp->rx_ring = kmalloc(RX_BUF_LEN + 16, GFP_KERNEL);
if (tp->tx_bufs == NULL || tp->rx_ring == NULL) {
if (tp->tx_bufs)
kfree(tp->tx_bufs);
if (rtl8129_debug > 0)
printk(KERN_ERR "%s: Couldn't allocate a %d byte
receive ring.\n",
dev->name, RX_BUF_LEN);
return -ENOMEM;
}
tp->tx_bufs could be NULL and tp->rx_ring could be allocated properly (if
some memory got released between the two kmalloc that can really happens).
So the check below should be added after the check for tp->tx_bufs:
if (tp->tx_ring)
kfree(tp->tx_ring);
Andrea
-
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/