Re: [PATCH net-next] liquidio: Remove unneeded cast from memory allocation
From: YueHaibing Date: Tue, 1 Sep 2020 22:11:15 +0800 > Remove unneeded return value cast. > This is detected by coccinelle. > > Signed-off-by: YueHaibing Applied.
[PATCH net-next] liquidio: Remove unneeded cast from memory allocation
Remove unneeded return value cast.
This is detected by coccinelle.
Signed-off-by: YueHaibing
---
drivers/net/ethernet/cavium/liquidio/octeon_droq.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_droq.c
b/drivers/net/ethernet/cavium/liquidio/octeon_droq.c
index 017169023cca..cf4fe5b17f8a 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_droq.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_droq.c
@@ -280,13 +280,10 @@ int octeon_init_droq(struct octeon_device *oct,
dev_dbg(>pci_dev->dev, "droq[%d]: num_desc: %d\n", q_no,
droq->max_count);
- droq->recv_buf_list = (struct octeon_recv_buffer *)
- vzalloc_node(array_size(droq->max_count, OCT_DROQ_RECVBUF_SIZE),
- numa_node);
+ droq->recv_buf_list = vzalloc_node(array_size(droq->max_count,
OCT_DROQ_RECVBUF_SIZE),
+ numa_node);
if (!droq->recv_buf_list)
- droq->recv_buf_list = (struct octeon_recv_buffer *)
- vzalloc(array_size(droq->max_count,
-OCT_DROQ_RECVBUF_SIZE));
+ droq->recv_buf_list = vzalloc(array_size(droq->max_count,
OCT_DROQ_RECVBUF_SIZE));
if (!droq->recv_buf_list) {
dev_err(>pci_dev->dev, "Output queue recv buf list alloc
failed\n");
goto init_droq_fail;
--
2.17.1
Re: [PATCH net-next] liquidio: Remove unneeded cast from memory allocation
在 2020/7/28 23:54, Joe Perches 写道:
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?
[]
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
@@ -1152,11 +1152,8 @@ octeon_register_dispatch_fn(struct octeon_device
*oct,
dev_dbg(>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(>pci_dev->dev,
- "No memory to add dispatch function\n");
return 1;
}
dispatch->opcode = combined_opcode;
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, ) {
list_del(temp);
- vfree(temp);
+ kfree(temp);
}
}
@@ -1152,13 +1152,10 @@ octeon_register_dispatch_fn(struct octeon_device *oct,
dev_dbg(>pci_dev->dev,
"Adding opcode to dispatch list linked list\n");
- dispatch = (struct octeon_dispatch *)
- vmalloc(sizeof(struct octeon_dispatch));
- if (!dispatch) {
- dev_err(>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;
.
Thanks for your suggestion. I just sent another patch for this.
"[PATCH net-next] liquidio: Replace vmalloc with kmalloc in
octeon_register_dispatch_fn()"
Re: [PATCH net-next] liquidio: Remove unneeded cast from memory allocation
> From: Joe Perches > Subject: [EXT] Re: [PATCH net-next] liquidio: Remove unneeded cast from > memory allocation > > On Tue, 2020-07-28 at 15:39 +, Derek Chickles wrote: > > I think that is fine as well. We just used vmalloc since there is no > > need for a physically contiguous piece of memory. > > Do any of the allocs in the driver actually need vmalloc? > This doesn't, but I believe some do, especially when they allocate lists.
Re: [PATCH net-next] liquidio: Remove unneeded cast from memory allocation
On Tue, 2020-07-28 at 15:39 +, Derek Chickles wrote: > I think that is fine as well. We just used vmalloc since there is no need > for a physically contiguous piece of memory. Do any of the allocs in the driver actually need vmalloc?
Re: [PATCH net-next] liquidio: Remove unneeded cast from memory allocation
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?
[]
> +++ b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
> @@ -1152,11 +1152,8 @@ octeon_register_dispatch_fn(struct octeon_device
> *oct,
>
> dev_dbg(>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(>pci_dev->dev,
> - "No memory to add dispatch function\n");
> return 1;
> }
> dispatch->opcode = combined_opcode;
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, ) {
list_del(temp);
- vfree(temp);
+ kfree(temp);
}
}
@@ -1152,13 +1152,10 @@ octeon_register_dispatch_fn(struct octeon_device *oct,
dev_dbg(>pci_dev->dev,
"Adding opcode to dispatch list linked list\n");
- dispatch = (struct octeon_dispatch *)
- vmalloc(sizeof(struct octeon_dispatch));
- if (!dispatch) {
- dev_err(>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;
Re: [PATCH net-next] liquidio: Remove unneeded cast from memory allocation
> From: wanghai (M)
> 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(>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(>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(>pci_dev->dev,
> - "No memory to add dispatch function\n");
> return 1;
> }
> dispatch->opcode = combined_opcode;
Looks fine to me.
Thanks,
Derek
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(>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.
if (!dispatch) {
dev_err(>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(>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(>pci_dev->dev,
- "No memory to add dispatch function\n");
return 1;
}
dispatch->opcode = combined_opcode;
.
Re: [PATCH net-next] liquidio: Remove unneeded cast from memory allocation
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(>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.
> > > if (!dispatch) {
> > > dev_err(>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.
Re: [PATCH net-next] liquidio: Remove unneeded cast from memory allocation
在 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(>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.
if (!dispatch) {
dev_err(>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
.
Re: [PATCH net-next] liquidio: Remove unneeded cast from memory allocation
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(>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.
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(>pci_dev->dev,
> "No memory to add dispatch function\n");
And this dev_err is unnecessary.
[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
---
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(>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(>pci_dev->dev,
"No memory to add dispatch function\n");
--
2.17.1
