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

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.

Signed-off-by: Wang Hai <redacted>
---
 drivers/net/ethernet/cavium/liquidio/octeon_device.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_device.c b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
index 934115d18..1473a669f 100644
--- 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));
 		if (!dispatch) {
 			dev_err(&oct->pci_dev->dev,
 				"No memory to add dispatch function\n");

-- 
2.17.1

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.

[]

[]

More the question is why this is vmalloc at all
as the structure size is very small.

Likely this should just be kmalloc.

drivers/net/ethernet/cavium/liquidio/octeon_device.h:struct octeon_dispatch {
drivers/net/ethernet/cavium/liquidio/octeon_device.h-   /** List head for this entry */
drivers/net/ethernet/cavium/liquidio/octeon_device.h-   struct list_head list;
drivers/net/ethernet/cavium/liquidio/octeon_device.h-
drivers/net/ethernet/cavium/liquidio/octeon_device.h-   /** The opcode for which the dispatch function & arg should be used */
drivers/net/ethernet/cavium/liquidio/octeon_device.h-   u16 opcode;
drivers/net/ethernet/cavium/liquidio/octeon_device.h-
drivers/net/ethernet/cavium/liquidio/octeon_device.h-   /** The function to be called for a packet received by the driver */
drivers/net/ethernet/cavium/liquidio/octeon_device.h-   octeon_dispatch_fn_t dispatch_fn;
drivers/net/ethernet/cavium/liquidio/octeon_device.h-
drivers/net/ethernet/cavium/liquidio/octeon_device.h-   /* The application specified argument to be passed to the above
drivers/net/ethernet/cavium/liquidio/octeon_device.h-    * function along with the received packet
drivers/net/ethernet/cavium/liquidio/octeon_device.h-    */
drivers/net/ethernet/cavium/liquidio/octeon_device.h-   void *arg;
drivers/net/ethernet/cavium/liquidio/octeon_device.h-}

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

And this dev_err is unnecessary.

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

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

  		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

.

On Tue, 2020-07-28 at 16:42 +0800, wanghai (M) wrote:

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

在 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;

.

On Tue, 2020-07-28 at 21:38 +0800, wanghai (M) wrote:

Thanks for your explanation. I got it.

Can it be modified like this?

[]

Yes, but the free also needs to be changed.

I think it's:
---
 drivers/net/ethernet/cavium/liquidio/octeon_device.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_device.c b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
index 934115d18488..4ee4cb946e1d 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_device.c

@@ -1056,7 +1056,7 @@ void octeon_delete_dispatch_list(struct octeon_device *oct)
 
 	list_for_each_safe(temp, tmp2, &freelist) {
 		list_del(temp);
-		vfree(temp);
+		kfree(temp);
 	}
 }
 

@@ -1152,13 +1152,10 @@ 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));
-		if (!dispatch) {
-			dev_err(&oct->pci_dev->dev,
-				"No memory to add dispatch function\n");
+		dispatch = kmalloc(sizeof(struct octeon_dispatch), GFP_KERNEL);
+		if (!dispatch)
 			return 1;
-		}
+
 		dispatch->opcode = combined_opcode;
 		dispatch->dispatch_fn = fn;
 		dispatch->arg = fn_arg;

在 2020/7/28 23:54, Joe Perches 写道:

Thanks for your suggestion. I just sent another patch for this.

"[PATCH net-next] liquidio: Replace vmalloc with kmalloc in 
octeon_register_dispatch_fn()"