[lustre-devel] Isn't there a memory leak here !!!

[lustre-devel] Isn't there a memory leak here !!!

[Sudheendra Sampath]

[-- Attachment #1.1: Type: text/plain, Size: 1353 bytes --]

Hi,

While doing transaction code analysis, I found the following in
lustre/target/tgt_lastrcvd.c.

    552 int tgt_new_client_cb_add(struct thandle *th, struct obd_export
*exp)
    553 {
    554         struct tgt_new_client_callback  *ccb;
    555         struct dt_txn_commit_cb         *dcb;
    556         int                              rc;
    557
*    558         OBD_ALLOC_PTR(ccb);*
    559         if (ccb == NULL)
    560                 return -ENOMEM;
    561
    562         ccb->lncc_exp = class_export_cb_get(exp);
    563
    564         dcb = &ccb->lncc_cb;
    565         dcb->dcb_func = tgt_cb_new_client;
    566         INIT_LIST_HEAD(&dcb->dcb_linkage);
    567         strlcpy(dcb->dcb_name, "tgt_cb_new_client",
sizeof(dcb->dcb_name));
    568
    569         rc = dt_trans_cb_add(th, dcb);
    570         if (rc) {
    571                 class_export_cb_put(exp);
*    572                 OBD_FREE_PTR(ccb);*
    573         }
    574         return rc;
    575 }

OBD_FREE_PTR() is in the condition block which means that the expectation
is dt_trans_cb_add() returns something "!= 0".

From the code, osd_trans_cb_add() and osp_trans_cb_add() returns zero
value.  So, my point is OBD_FREE_PTR() should be outside the condition
block.

Please correct me if my understanding is incorrect.

-- 
Regards

Sudheendra Sampath

[-- Attachment #1.2: Type: text/html, Size: 1945 bytes --]

[-- Attachment #2: Type: text/plain, Size: 165 bytes --]

_______________________________________________
lustre-devel mailing list
lustre-devel@lists.lustre.org
http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

Re: [lustre-devel] Isn't there a memory leak here !!!

[NeilBrown]

[-- Attachment #1.1: Type: text/plain, Size: 2159 bytes --]

On Wed, Jan 20 2021, Sudheendra Sampath wrote:

> Hi,
>
> While doing transaction code analysis, I found the following in
> lustre/target/tgt_lastrcvd.c.
>
>     552 int tgt_new_client_cb_add(struct thandle *th, struct obd_export
> *exp)
>     553 {
>     554         struct tgt_new_client_callback  *ccb;
>     555         struct dt_txn_commit_cb         *dcb;
>     556         int                              rc;
>     557
> *    558         OBD_ALLOC_PTR(ccb);*
>     559         if (ccb == NULL)
>     560                 return -ENOMEM;
>     561
>     562         ccb->lncc_exp = class_export_cb_get(exp);
>     563
>     564         dcb = &ccb->lncc_cb;
>     565         dcb->dcb_func = tgt_cb_new_client;
>     566         INIT_LIST_HEAD(&dcb->dcb_linkage);
>     567         strlcpy(dcb->dcb_name, "tgt_cb_new_client",
> sizeof(dcb->dcb_name));
>     568
>     569         rc = dt_trans_cb_add(th, dcb);
>     570         if (rc) {
>     571                 class_export_cb_put(exp);
> *    572                 OBD_FREE_PTR(ccb);*
>     573         }
>     574         return rc;
>     575 }
>
> OBD_FREE_PTR() is in the condition block which means that the expectation
> is dt_trans_cb_add() returns something "!= 0".
>
> From the code, osd_trans_cb_add() and osp_trans_cb_add() returns zero
> value.  So, my point is OBD_FREE_PTR() should be outside the condition
> block.
>
> Please correct me if my understanding is incorrect.

If dt_trans_cb_add() returns zero, then 'dcb' has been added to some
list of transactions.  dcb is a pointer to a 'struct dt_txn_commit_cb'
structure embedded inside 'ccb'.

So when rc==0, the memory allocated for ccb is now attached to a list of
transactions, so it would be wrong to free it.  If the attachment
(dt_trans_cb_add()) failed, then it needs to be freed.

So I think the code is correct as it stands.

Thanks,
NeilBrown


>
> -- 
> Regards
>
> Sudheendra Sampath
> _______________________________________________
> lustre-devel mailing list
> lustre-devel@lists.lustre.org
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]

[-- Attachment #2: Type: text/plain, Size: 165 bytes --]

_______________________________________________
lustre-devel mailing list
lustre-devel@lists.lustre.org
http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org