On 9/16/21 2:56 PM, Lorenzo Bianconi wrote:
> Introduce memory accounting for data structures in ovn-controller
> if_status_mgr module.
>
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> ---
Hi Lorenzo,
> controller/if-status.c | 23 +++++++++++++++++++++++
> controller/if-status.h | 3 +++
> controller/ovn-controller.c | 1 +
> 3 files changed, 27 insertions(+)
>
> diff --git a/controller/if-status.c b/controller/if-status.c
> index 08fb50b87..cd544f9ff 100644
> --- a/controller/if-status.c
> +++ b/controller/if-status.c
> @@ -18,6 +18,7 @@
> #include "binding.h"
> #include "if-status.h"
> #include "ofctrl-seqno.h"
> +#include "simap.h"
>
> #include "lib/hmapx.h"
> #include "lib/util.h"
> @@ -85,6 +86,8 @@ struct ovs_iface {
> */
> };
>
> +static uint64_t ifaces_usage;
> +
> /* State machine manager for all local OVS interfaces. */
> struct if_status_mgr {
> /* All local interfaces, mapping from 'iface-id' to 'struct ovs_iface'.
> */
> @@ -345,6 +348,8 @@ ovs_iface_create(struct if_status_mgr *mgr, const char
> *iface_id,
> iface->id = xstrdup(iface_id);
> shash_add(&mgr->ifaces, iface_id, iface);
> ovs_iface_set_state(mgr, iface, state);
> + ifaces_usage += (strlen(iface_id) + sizeof *iface +
> + sizeof(struct shash_node));
Looks like this is not really accurate, because shash_add(shash, name,
data) will xstrdup(name), so we need to account for iface_id twice.
> return iface;
> }
>
> @@ -355,6 +360,8 @@ ovs_iface_destroy(struct if_status_mgr *mgr, struct
> ovs_iface *iface)
> if_state_names[iface->state]);
> hmapx_find_and_delete(&mgr->ifaces_per_state[iface->state], iface);
> shash_find_and_delete(&mgr->ifaces, iface->id);
> + ifaces_usage -= (strlen(iface->id) + sizeof *iface +
> + sizeof(struct shash_node));
Same remark here about iface->id that needs to be accounted for twice.
Maybe we should be using shash_add_nocopy() instead when adding to the
shash.
Also, to avoid duplicating code and potentially accounting for memory
differently when allocating vs freeing, should we maybe add a helper
function to compute this size?
What do you think?
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
