[PATCH 1/3] staging: et131x: simplify rx dma code

Dan Carpenter dan.carpenter at oracle.com
Wed Nov 27 12:04:20 UTC 2013


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 at oracle.com> 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



More information about the devel mailing list