On Wed, Nov 27, 2013 at 07:37:30PM +0800, ZHAO Gang wrote: > On Wed, Nov 27, 2013 at 5:02 PM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > > On Wed, Nov 27, 2013 at 03:45:12PM +0800, ZHAO Gang wrote: > >> @@ -2208,8 +2204,11 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter) > >> rx_ring = &adapter->rx_ring; > >> > >> /* Alloc memory for the lookup table */ > >> - rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); > >> - rx_ring->fbr[1] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); > >> + rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup) * 2, GFP_KERNEL); > >> + if (!rx_ring->fbr[0]) > >> + return -ENOMEM; > >> + > >> + rx_ring->fbr[1] = rx_ring->fbr[0] + 1; > >> > >> /* The first thing we will do is configure the sizes of the buffer > >> * rings. These will change based on jumbo packet support. Larger > > > > I can't make myself review any further beyond this point... Please > > don't do this nonsense. > > > > I don't think it is nonsense. The original code doesn't have error > check on this two kmalloc, I combine them to one so I only need to add > one error check on it. Just do two allocations and two checks unless you have benchmark numbers to show that it faster. Making the code so complicated for no reason is a wrong thing. regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
