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:

quoted

在 2020/7/25 5:29, Joe Perches 写道:

quoted

On Fri, 2020-07-24 at 21:00 +0800, Wang Hai wrote:

quoted

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.

[]

quoted

diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_device.c b/drivers/net/ethernet/cavium/liquidio/octeon_device.c

[]

quoted

@@ -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.

quoted

quoted

   		if (!dispatch) {
   			dev_err(&oct->pci_dev->dev,
   				"No memory to add dispatch function\n");

And this dev_err is unnecessary.

I don't understand why dev_err is not needed here. We can easily know
that an error has occurred here through dev_err

Memory allocation failures without __GFP_NOWARN. already
do a dump_stack to show the location of the code that
could not successfully allocate memory.

Thanks for your explanation. I got it.

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;

.