> From: wanghai (M) <wangha...@huawei.com>
> 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
