The original code allocate rx dma memory in several dma_alloc_coherent calls,
which causes some problems:
1. since dma_alloc_coherent allocate at least one page memory, it wastes some
memory when allocation size is smaller than one page.
2. it causes et131x_rx_dma_memory_free as complex as et131x_rx_dma_memory_alloc
Instead, allocate all rx dma memory in one dma_alloc_coherent call makes less code,
makes it easy to handle dma allocation error, and makes et131x_rx_dma_memory_free
as simple as it could be.
Also add error check to kmalloc.
Signed-off-by: ZHAO Gang <gamerh2o@xxxxxxxxx>
---
drivers/staging/et131x/et131x.c | 202 ++++++++++++----------------------------
1 file changed, 59 insertions(+), 143 deletions(-)
diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index 27da1db..a9ae1f3 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -304,6 +304,7 @@ struct rx_ring {
u32 num_ready_recv;
u32 num_rfd;
+ u32 dma_size;
bool unfinished_receives;
};
@@ -2186,21 +2187,15 @@ static inline u32 bump_free_buff_ring(u32 *free_buff_ring, u32 limit)
return tmp_free_buff_ring;
}
-/* et131x_rx_dma_memory_alloc
- * @adapter: pointer to our private adapter structure
- *
- * Returns 0 on success and errno on failure (as defined in errno.h)
- *
- * Allocates Free buffer ring 1 for sure, free buffer ring 0 if required,
- * and the Packet Status Ring.
- */
static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter)
{
u8 id;
u32 i, j;
- u32 bufsize;
- u32 pktstat_ringsize;
- u32 fbr_chunksize;
+ u32 dma_size;
+ u32 fbr_size;
+ u32 pktstat_ring_size;
+ dma_addr_t dma_addr;
+ void *virt_addr;
struct rx_ring *rx_ring;
struct fbr_lookup *fbr;
@@ -2209,7 +2204,14 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter)
/* Alloc memory for the lookup table */
rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL);
+ if (!rx_ring->fbr[0])
+ return -ENOMEM;
+
rx_ring->fbr[1] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL);
+ if (!rx_ring->fbr[1]) {
+ kfree(rx_ring->fbr[0]);
+ return -ENOMEM;
+ }
/* The first thing we will do is configure the sizes of the buffer
* rings. These will change based on jumbo packet support. Larger
@@ -2246,48 +2248,45 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter)
rx_ring->fbr[1]->num_entries = 128;
}
- adapter->rx_ring.psr_num_entries =
- adapter->rx_ring.fbr[0]->num_entries +
- adapter->rx_ring.fbr[1]->num_entries;
+ rx_ring->psr_num_entries =
+ rx_ring->fbr[0]->num_entries + rx_ring->fbr[1]->num_entries;
+ pktstat_ring_size =
+ sizeof(struct pkt_stat_desc) * rx_ring->psr_num_entries;
+
+ dma_size = sizeof(struct fbr_desc) * rx_ring->psr_num_entries;
+ dma_size += pktstat_ring_size;
+ dma_size += sizeof(struct rx_status_block);
for (id = 0; id < NUM_FBRS; id++) {
- fbr = rx_ring->fbr[id];
+ dma_size += FBR_CHUNKS * rx_ring->fbr[id]->buffsize;
+ }
- /* Allocate an area of memory for Free Buffer Ring */
- bufsize = sizeof(struct fbr_desc) * fbr->num_entries;
- fbr->ring_virtaddr = dma_alloc_coherent(&adapter->pdev->dev,
- bufsize,
- &fbr->ring_physaddr,
- GFP_KERNEL);
+ rx_ring->dma_size = dma_size;
- if (!fbr->ring_virtaddr) {
- dev_err(&adapter->pdev->dev,
- "Cannot alloc memory for Free Buffer Ring %d\n", id);
- return -ENOMEM;
- }
+ virt_addr = dma_alloc_coherent(&adapter->pdev->dev,
+ dma_size,
+ &dma_addr,
+ GFP_KERNEL);
+ if (!virt_addr) {
+ kfree(rx_ring->fbr[0]);
+ kfree(rx_ring->fbr[1]);
+ dev_err(&adapter->pdev->dev,
+ "Cannot allocate memory for Rx ring\n");
+ return -ENOMEM;
}
+ /* Now we distribute the pie */
for (id = 0; id < NUM_FBRS; id++) {
fbr = rx_ring->fbr[id];
- fbr_chunksize = (FBR_CHUNKS * fbr->buffsize);
+ fbr->ring_virtaddr = virt_addr;
+ fbr->ring_physaddr = dma_addr;
+ fbr_size = sizeof(struct fbr_desc) * fbr->num_entries;
+ virt_addr += fbr_size;
+ dma_addr += fbr_size;
for (i = 0; i < (fbr->num_entries / FBR_CHUNKS); i++) {
- dma_addr_t fbr_tmp_physaddr;
-
- fbr->mem_virtaddrs[i] =
- dma_alloc_coherent(&adapter->pdev->dev,
- fbr_chunksize,
- &fbr->mem_physaddrs[i],
- GFP_KERNEL);
-
- if (!fbr->mem_virtaddrs[i]) {
- dev_err(&adapter->pdev->dev,
- "Could not alloc memory\n");
- return -ENOMEM;
- }
-
- /* See NOTE in "Save Physical Address" comment above */
- fbr_tmp_physaddr = fbr->mem_physaddrs[i];
+ fbr->mem_virtaddrs[i] = virt_addr;
+ fbr->mem_physaddrs[i] = dma_addr;
for (j = 0; j < FBR_CHUNKS; j++) {
u32 index = (i * FBR_CHUNKS) + j;
@@ -2301,52 +2300,28 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter)
/* now store the physical address in the
* descriptor so the device can access it
*/
- fbr->bus_high[index] =
- upper_32_bits(fbr_tmp_physaddr);
- fbr->bus_low[index] =
- lower_32_bits(fbr_tmp_physaddr);
-
- fbr_tmp_physaddr += fbr->buffsize;
+ fbr->bus_high[index] = upper_32_bits(dma_addr);
+ fbr->bus_low[index] = lower_32_bits(dma_addr);
+ dma_addr += fbr->buffsize;
+ virt_addr += fbr->buffsize;
}
}
}
- /* Allocate an area of memory for FIFO of Packet Status ring entries */
- pktstat_ringsize =
- sizeof(struct pkt_stat_desc) * adapter->rx_ring.psr_num_entries;
+ rx_ring->ps_ring_virtaddr = virt_addr;
+ rx_ring->ps_ring_physaddr = dma_addr;
+ virt_addr += pktstat_ring_size;
+ dma_addr += pktstat_ring_size;
- rx_ring->ps_ring_virtaddr = dma_alloc_coherent(&adapter->pdev->dev,
- pktstat_ringsize,
- &rx_ring->ps_ring_physaddr,
- GFP_KERNEL);
+ rx_ring->rx_status_block = virt_addr;
+ rx_ring->rx_status_bus = dma_addr;
- if (!rx_ring->ps_ring_virtaddr) {
- dev_err(&adapter->pdev->dev,
- "Cannot alloc memory for Packet Status Ring\n");
- return -ENOMEM;
- }
pr_info("Packet Status Ring %llx\n",
(unsigned long long) rx_ring->ps_ring_physaddr);
+ pr_info("Receive Status Ring %llx\n",
+ (unsigned long long)rx_ring->rx_status_bus);
- /* NOTE : dma_alloc_coherent(), used above to alloc DMA regions,
- * ALWAYS returns SAC (32-bit) addresses. If DAC (64-bit) addresses
- * are ever returned, make sure the high part is retrieved here before
- * storing the adjusted address.
- */
-
- /* Allocate an area of memory for writeback of status information */
- rx_ring->rx_status_block = dma_alloc_coherent(&adapter->pdev->dev,
- sizeof(struct rx_status_block),
- &rx_ring->rx_status_bus,
- GFP_KERNEL);
- if (!rx_ring->rx_status_block) {
- dev_err(&adapter->pdev->dev,
- "Cannot alloc memory for Status Block\n");
- return -ENOMEM;
- }
rx_ring->num_rfd = NIC_DEFAULT_NUM_RFD;
- pr_info("PRS %llx\n", (unsigned long long)rx_ring->rx_status_bus);
-
/* The RFDs are going to be put on lists later on, so initialize the
* lists now.
*/
@@ -2354,18 +2329,10 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter)
return 0;
}
-/* et131x_rx_dma_memory_free - Free all memory allocated within this module.
- * @adapter: pointer to our private adapter structure
- */
static void et131x_rx_dma_memory_free(struct et131x_adapter *adapter)
{
- u8 id;
- u32 index;
- u32 bufsize;
- u32 pktstat_ringsize;
struct rfd *rfd;
struct rx_ring *rx_ring;
- struct fbr_lookup *fbr;
/* Setup some convenience pointers */
rx_ring = &adapter->rx_ring;
@@ -2382,67 +2349,16 @@ static void et131x_rx_dma_memory_free(struct et131x_adapter *adapter)
kfree(rfd);
}
- /* Free Free Buffer Rings */
- for (id = 0; id < NUM_FBRS; id++) {
- fbr = rx_ring->fbr[id];
-
- if (!fbr->ring_virtaddr)
- continue;
-
- /* First the packet memory */
- for (index = 0; index < (fbr->num_entries / FBR_CHUNKS);
- index++) {
- if (fbr->mem_virtaddrs[index]) {
- bufsize = fbr->buffsize * FBR_CHUNKS;
-
- dma_free_coherent(&adapter->pdev->dev,
- bufsize,
- fbr->mem_virtaddrs[index],
- fbr->mem_physaddrs[index]);
-
- fbr->mem_virtaddrs[index] = NULL;
- }
- }
-
- bufsize = sizeof(struct fbr_desc) * fbr->num_entries;
-
- dma_free_coherent(&adapter->pdev->dev,
- bufsize,
- fbr->ring_virtaddr,
- fbr->ring_physaddr);
-
- fbr->ring_virtaddr = NULL;
- }
-
- /* Free Packet Status Ring */
- if (rx_ring->ps_ring_virtaddr) {
- pktstat_ringsize = sizeof(struct pkt_stat_desc) *
- adapter->rx_ring.psr_num_entries;
-
+ if (rx_ring->fbr[0]->ring_virtaddr)
dma_free_coherent(&adapter->pdev->dev,
- pktstat_ringsize,
- rx_ring->ps_ring_virtaddr,
- rx_ring->ps_ring_physaddr);
-
- rx_ring->ps_ring_virtaddr = NULL;
- }
-
- /* Free area of memory for the writeback of status information */
- if (rx_ring->rx_status_block) {
- dma_free_coherent(&adapter->pdev->dev,
- sizeof(struct rx_status_block),
- rx_ring->rx_status_block,
- rx_ring->rx_status_bus);
-
- rx_ring->rx_status_block = NULL;
- }
+ rx_ring->dma_size,
+ rx_ring->fbr[0]->ring_virtaddr,
+ rx_ring->fbr[0]->ring_physaddr);
- /* Free the FBR Lookup Table */
kfree(rx_ring->fbr[0]);
kfree(rx_ring->fbr[1]);
- /* Reset Counters */
- rx_ring->num_ready_recv = 0;
+ memset(rx_ring, 0, sizeof(struct rx_ring));
}
/* et131x_init_recv - Initialize receive data structures.
--
1.8.3.1
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
