Re: [PATCH net-next] liquidio: Remove unneeded cast from memory allocation

[Derek Chickles]

> From: wanghai (M) <wanghai38@xxxxxxxxxx>
> Subject: [EXT] Re: [PATCH net-next] liquidio: Remove unneeded cast from
> memory allocation
> 
> å 2020/7/28 17:11, Joe Perches åé:
> > On Tue, 2020-07-28 at 16:42 +0800, wanghai (M) wrote:
> >> å 2020/7/25 5:29, Joe Perches åé:
> >>> On Fri, 2020-07-24 at 21:00 +0800, Wang Hai wrote:
> >>>> Remove casting the values returned by memory allocation function.
> >>>>
> >>>> Coccinelle emits WARNING:
> >>>>
> >>>> ./drivers/net/ethernet/cavium/liquidio/octeon_device.c:1155:14-36:
> WARNING:
> >>>>    casting value returned by memory allocation function to (struct
> octeon_dispatch *) is useless.
> >>> []
> >>>> diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_device.c
> >>>> b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
> >>> []
> >>>> @@ -1152,8 +1152,7 @@ octeon_register_dispatch_fn(struct
> >>>> octeon_device *oct,
> >>>>
> >>>>    		dev_dbg(&oct->pci_dev->dev,
> >>>>    			"Adding opcode to dispatch list linked list\n");
> >>>> -		dispatch = (struct octeon_dispatch *)
> >>>> -			   vmalloc(sizeof(struct octeon_dispatch));
> >>>> +		dispatch = vmalloc(sizeof(struct octeon_dispatch));
> >>> More the question is why this is vmalloc at all as the structure
> >>> size is very small.
> >>>
> >>> Likely this should just be kmalloc.
> >>>
> >>>
> >> Thanks for your advice.  It is indeed best to use kmalloc here.

I think that is fine as well. We just used vmalloc since there is no need
for a physically contiguous piece of memory.

...

> 
> Can it be modified like this?
> 
> --- a/drivers/net/ethernet/cavium/liquidio/octeon_device.c
> +++ b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
> @@ -1152,11 +1152,8 @@ octeon_register_dispatch_fn(struct
> octeon_device *oct,
> 
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ dev_dbg(&oct->pci_dev->dev,
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "Adding opcode to dispatch list linked list\n");
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ dispatch = (struct octeon_dispatch *)
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ vmalloc(sizeof(struct octeon_dispatch));
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ dispatch = kmalloc(sizeof(struct octeon_dispatch),
> GFP_KERNEL);
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (!dispatch) {
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ dev_err(&oct->pci_dev->dev,
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "No memory to add dispatch function\n");
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ return 1;
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ }
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ dispatch->opcode = combined_opcode;

Looks fine to me.

Thanks,
Derek