* [PATCH net-next] qed: Set error code for allocation failures
@ 2017-10-27 6:40 Dan Carpenter
2017-10-27 9:32 ` Yunsheng Lin
2017-10-29 7:50 ` Kalderon, Michal
0 siblings, 2 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-10-27 6:40 UTC (permalink / raw)
To: Ariel Elior; +Cc: everest-linux-l2, netdev, linux-kernel, kernel-janitors
There are several places where we accidentally return success when
kcalloc() fails.
Fixes: fcb39f6c10b2 ("qed: Add mpa buffer descriptors for storing and processing mpa fpdus")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
index 409041eab189..6366f2ef82b7 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
@@ -2585,7 +2585,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
struct qed_ll2_cbs cbs;
u32 mpa_buff_size;
u16 n_ooo_bufs;
- int rc = 0;
+ int rc;
int i;
iwarp_info = &p_hwfn->p_rdma_info->iwarp;
@@ -2696,6 +2696,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
if (rc)
goto err;
+ rc = -ENOMEM;
iwarp_info->partial_fpdus = kcalloc((u16)p_hwfn->p_rdma_info->num_qps,
sizeof(*iwarp_info->partial_fpdus),
GFP_KERNEL);
@@ -2724,7 +2725,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
for (i = 0; i < data.input.rx_num_desc; i++)
list_add_tail(&iwarp_info->mpa_bufs[i].list_entry,
&iwarp_info->mpa_buf_list);
- return rc;
+ return 0;
err:
qed_iwarp_ll2_stop(p_hwfn, p_ptt);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] qed: Set error code for allocation failures
2017-10-27 6:40 [PATCH net-next] qed: Set error code for allocation failures Dan Carpenter
@ 2017-10-27 9:32 ` Yunsheng Lin
2017-10-27 11:52 ` Dan Carpenter
2017-10-29 7:50 ` Kalderon, Michal
1 sibling, 1 reply; 6+ messages in thread
From: Yunsheng Lin @ 2017-10-27 9:32 UTC (permalink / raw)
To: Dan Carpenter, Ariel Elior
Cc: everest-linux-l2, netdev, linux-kernel, kernel-janitors
Hi, Dan
On 2017/10/27 14:40, Dan Carpenter wrote:
> There are several places where we accidentally return success when
> kcalloc() fails.
>
> Fixes: fcb39f6c10b2 ("qed: Add mpa buffer descriptors for storing and processing mpa fpdus")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> index 409041eab189..6366f2ef82b7 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> @@ -2585,7 +2585,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
> struct qed_ll2_cbs cbs;
> u32 mpa_buff_size;
> u16 n_ooo_bufs;
> - int rc = 0;
> + int rc;
> int i;
>
> iwarp_info = &p_hwfn->p_rdma_info->iwarp;
> @@ -2696,6 +2696,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
> if (rc)
> goto err;
>
> + rc = -ENOMEM;
> iwarp_info->partial_fpdus = kcalloc((u16)p_hwfn->p_rdma_info->num_qps,
> sizeof(*iwarp_info->partial_fpdus),
> GFP_KERNEL);
Does the memory allocated here need to be freed when error happens below?
> @@ -2724,7 +2725,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
> for (i = 0; i < data.input.rx_num_desc; i++)
> list_add_tail(&iwarp_info->mpa_bufs[i].list_entry,
> &iwarp_info->mpa_buf_list);
> - return rc;
> + return 0;
> err:
> qed_iwarp_ll2_stop(p_hwfn, p_ptt);
>
>
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] qed: Set error code for allocation failures
2017-10-27 9:32 ` Yunsheng Lin
@ 2017-10-27 11:52 ` Dan Carpenter
2017-10-28 5:50 ` Yunsheng Lin
2017-10-29 7:45 ` Kalderon, Michal
0 siblings, 2 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-10-27 11:52 UTC (permalink / raw)
To: Yunsheng Lin
Cc: Ariel Elior, everest-linux-l2, netdev, linux-kernel, kernel-janitors
On Fri, Oct 27, 2017 at 05:32:42PM +0800, Yunsheng Lin wrote:
> Hi, Dan
>
> On 2017/10/27 14:40, Dan Carpenter wrote:
> > There are several places where we accidentally return success when
> > kcalloc() fails.
> >
> > Fixes: fcb39f6c10b2 ("qed: Add mpa buffer descriptors for storing and processing mpa fpdus")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > diff --git a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> > index 409041eab189..6366f2ef82b7 100644
> > --- a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> > +++ b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> > @@ -2585,7 +2585,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
> > struct qed_ll2_cbs cbs;
> > u32 mpa_buff_size;
> > u16 n_ooo_bufs;
> > - int rc = 0;
> > + int rc;
> > int i;
> >
> > iwarp_info = &p_hwfn->p_rdma_info->iwarp;
> > @@ -2696,6 +2696,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
> > if (rc)
> > goto err;
> >
> > + rc = -ENOMEM;
> > iwarp_info->partial_fpdus = kcalloc((u16)p_hwfn->p_rdma_info->num_qps,
> > sizeof(*iwarp_info->partial_fpdus),
> > GFP_KERNEL);
>
> Does the memory allocated here need to be freed when error happens below?
>
Hm... I think you're right that it leaks. Also I'm confused by the
qed_iwarp_ll2_alloc_buffers() allocation. The comment in there says
that /* buffers will be deallocated by qed_ll2 */ but qed_ll2 is not
a function name or something which is useful to grep.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] qed: Set error code for allocation failures
2017-10-27 11:52 ` Dan Carpenter
@ 2017-10-28 5:50 ` Yunsheng Lin
2017-10-29 7:45 ` Kalderon, Michal
1 sibling, 0 replies; 6+ messages in thread
From: Yunsheng Lin @ 2017-10-28 5:50 UTC (permalink / raw)
To: Dan Carpenter
Cc: Ariel Elior, everest-linux-l2, netdev, linux-kernel, kernel-janitors
Hi, Dan
On 2017/10/27 19:52, Dan Carpenter wrote:
> On Fri, Oct 27, 2017 at 05:32:42PM +0800, Yunsheng Lin wrote:
>> Hi, Dan
>>
>> On 2017/10/27 14:40, Dan Carpenter wrote:
>>> There are several places where we accidentally return success when
>>> kcalloc() fails.
>>>
>>> Fixes: fcb39f6c10b2 ("qed: Add mpa buffer descriptors for storing and processing mpa fpdus")
>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>
>>> diff --git a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
>>> index 409041eab189..6366f2ef82b7 100644
>>> --- a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
>>> +++ b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
>>> @@ -2585,7 +2585,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
>>> struct qed_ll2_cbs cbs;
>>> u32 mpa_buff_size;
>>> u16 n_ooo_bufs;
>>> - int rc = 0;
>>> + int rc;
>>> int i;
>>>
>>> iwarp_info = &p_hwfn->p_rdma_info->iwarp;
>>> @@ -2696,6 +2696,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
>>> if (rc)
>>> goto err;
>>>
>>> + rc = -ENOMEM;
>>> iwarp_info->partial_fpdus = kcalloc((u16)p_hwfn->p_rdma_info->num_qps,
>>> sizeof(*iwarp_info->partial_fpdus),
>>> GFP_KERNEL);
>>
>> Does the memory allocated here need to be freed when error happens below?
>>
>
> Hm... I think you're right that it leaks. Also I'm confused by the
> qed_iwarp_ll2_alloc_buffers() allocation. The comment in there says
> that /* buffers will be deallocated by qed_ll2 */ but qed_ll2 is not
> a function name or something which is useful to grep.
Yes, I am confused by it too.
Even in qed_iwarp_ll2_alloc_buffers, if kzcalloc failed, it do not clean
up the memory allocated by pre kzcalloc.
>
> regards,
> dan carpenter
>
>
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] qed: Set error code for allocation failures
2017-10-27 11:52 ` Dan Carpenter
2017-10-28 5:50 ` Yunsheng Lin
@ 2017-10-29 7:45 ` Kalderon, Michal
1 sibling, 0 replies; 6+ messages in thread
From: Kalderon, Michal @ 2017-10-29 7:45 UTC (permalink / raw)
To: Dan Carpenter, Yunsheng Lin
Cc: Elior, Ariel, Dept-Eng Everest Linux L2, netdev, linux-kernel,
kernel-janitors
From: Dan Carpenter <dan.carpenter@oracle.com>
Sent: Friday, October 27, 2017 2:52 PM
>On Fri, Oct 27, 2017 at 05:32:42PM +0800, Yunsheng Lin wrote:
>> > iwarp_info = &p_hwfn->p_rdma_info->iwarp;
>> > @@ -2696,6 +2696,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
>> > if (rc)
>> > goto err;
>> >
>> > + rc = -ENOMEM;
>> > iwarp_info->partial_fpdus = kcalloc((u16)p_hwfn->p_rdma_info->num_qps,
>> > sizeof(*iwarp_info->partial_fpdus),
>> > GFP_KERNEL);
>>
>> Does the memory allocated here need to be freed when error happens below?
>>
>
>Hm... I think you're right that it leaks. Also I'm confused by the
>qed_iwarp_ll2_alloc_buffers() allocation. The comment in there says
>that /* buffers will be deallocated by qed_ll2 */ but qed_ll2 is not
>a function name or something which is useful to grep.
Thanks Dan, partial_fpdus is released during qed_iwarp_resc_free, called from qed_rdma_resc_free
called on qed_rdma_stop and on failure during qed_rdma_start.
Regarding ll2 buffers. For each successfully allocated buffer we call
qed_iwarp_ll2_post_rx->qed_ll2_post_rx_buffer.
These buffers will get released and freed when we call qed_ll2_terminate_connection
called from qed_iwarp_ll2_stop ( which is called during qed_iwarp_ll2_start on error ).
thanks,
Michal
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] qed: Set error code for allocation failures
2017-10-27 6:40 [PATCH net-next] qed: Set error code for allocation failures Dan Carpenter
2017-10-27 9:32 ` Yunsheng Lin
@ 2017-10-29 7:50 ` Kalderon, Michal
1 sibling, 0 replies; 6+ messages in thread
From: Kalderon, Michal @ 2017-10-29 7:50 UTC (permalink / raw)
To: Dan Carpenter, Elior, Ariel
Cc: Dept-Eng Everest Linux L2, netdev, linux-kernel, kernel-janitors
From: Dan Carpenter <dan.carpenter@oracle.com>
Sent: Friday, October 27, 2017 9:40 AM
> There are several places where we accidentally return success when
> kcalloc() fails.
>
> Fixes: fcb39f6c10b2 ("qed: Add mpa buffer descriptors for storing and processing mpa fpdus")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> index 409041eab189..6366f2ef82b7 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> @@ -2585,7 +2585,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
> struct qed_ll2_cbs cbs;
> u32 mpa_buff_size;
> u16 n_ooo_bufs;
> - int rc = 0;
> + int rc;
> int i;
>
> iwarp_info = &p_hwfn->p_rdma_info->iwarp;
> @@ -2696,6 +2696,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
> if (rc)
> goto err;
>
> + rc = -ENOMEM;
> iwarp_info->partial_fpdus = kcalloc((u16)p_hwfn->p_rdma_info->num_qps,
> sizeof(*iwarp_info->partial_fpdus),
> GFP_KERNEL);
> @@ -2724,7 +2725,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
> for (i = 0; i < data.input.rx_num_desc; i++)
> list_add_tail(&iwarp_info->mpa_bufs[i].list_entry,
> &iwarp_info->mpa_buf_list);
> - return rc;
> + return 0;
> err:
> qed_iwarp_ll2_stop(p_hwfn, p_ptt);
Thanks Dan,
Acked-by: Michal Kalderon <michal.kalderon@cavium.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-29 7:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-27 6:40 [PATCH net-next] qed: Set error code for allocation failures Dan Carpenter
2017-10-27 9:32 ` Yunsheng Lin
2017-10-27 11:52 ` Dan Carpenter
2017-10-28 5:50 ` Yunsheng Lin
2017-10-29 7:45 ` Kalderon, Michal
2017-10-29 7:50 ` Kalderon, Michal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).