When is gf_mem_acct_fini() called and actually does something?

[mykaul]

gf_mem_acct_fini() is checking if acct->refcnt is 0 and if so, frees the mem_acct structure . However, acct->refcnt is initialized to 1, is always incremented on allocation and is decremented in gf_mem_acct_fini() - so I don't see how it'll ever reach 0.
Am I missing a use case? If it's not useful, can we remove it?

[mykaul]

@mohit84 - might be relevant for your lockless memory accounting patch.

[mohit84]

gf_mem_acct_fini() is checking if acct->refcnt is 0 and if so, frees the mem_acct structure . However, acct->refcnt is initialized to 1, is always incremented on allocation and is decremented in gf_mem_acct_fini() - so I don't see how it'll ever reach 0.
Am I missing a use case? If it's not useful, can we remove it?

I think you are saying about xlator_mem_acct_unref, i am not able to find any function (gf_mem_acct_fini).The xlator function is applicable to release mem_acct, we do call xlator_mem_acct_unref two times 1) at the time of calling __gf_free and 2) at the time
of calling free xlator object xlator_memrec_free so i don't think we have any issue of cleanup mem_acct.

[mykaul]

Thanks, I got slightly confused, and you've corrected it well, but I still don't see how in _gf_free() it'll ever clear anything, let me try again:

1. In xlator_mem_acct_init() we set refcnt to 1:

GF_ATOMIC_INIT(xl->mem_acct->refcnt, 1);

2. On the first allocation of a specific type, we increment it again, in gf_mem_set_acct_info() function:

        /* We only take a reference for each memory type used, not for each
         * allocation. This minimizes the use of atomic operations. */
        if (new_ref) {
            GF_ATOMIC_INC(mem_acct->refcnt);
        }

3. When we __gf_free(), if it's the last allocation of that specific type, we call xlator_mem_acct_unref():

    if (last_ref) {
        xlator_mem_acct_unref(mem_acct);
    }

Which will decrement it by 1, and see if it is equal 0 - but why would it be - it was never 0 in the first place.
I therefore don't see when it'll ever get cleaned as part of freeing memory. OTOH, as part of xlator_memrec_free(), if all memory that was allocated was properly free'd, I do see that it should work.

[mohit84]

xlator_mem_acct_unref

Yes you are correct xlator_memrec_free is the function that we call while freeing xlator object.We don't expect during call of last GF_FREE mem_accounting object should be cleanup so we are taking 1 reference during xlator_mem_acct_init.Yes if all memory block will not free in that case it will be a leak.

[mykaul]

So we can remove:

    if (!num_allocs) {
        xlator_mem_acct_unref(mem_acct);

from __gf_free()

[mohit84]

No we can't , currently cli code path does not handle memory allocation/deallocation correctly.Because the cli is a instant process so we don't care it.For others we do take reference (1 call xlaotor_mem_acct_init, 2nd during call of __gf_calloc/malloc), so we need require unref at both places or we can remove from _gf_free only while we are not taking reference from __gf_calloc/malloc. To do it we have to handle allocation/deallocation at cli properly.

[mykaul]

I don't think we care deeply about the CLI referencing or memory accounting. I think we can perhaps do the decrement, if needed, but not call the unref, which is never going to be useful in the __gf_free() flow.

[mohit84]

That we can do it.