| Message ID | 152331919110.20574.2359248862392897661.stgit@djiang5-desk3.ch.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 9, 2018 at 5:13 PM, Dave Jiang <dave.jiang@intel.com> wrote: > daxctl list is not calling daxctl_unref() when executed succesfully. At the same > time, daxctl_region_unref() is not being called when daxctl_unref() executes. > Valgrind is reporting unfreed memory. Adding the appropriate calls to make sure > all memory allocated are freed for daxctl list. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > --- > daxctl/lib/libdaxctl.c | 7 +++++++ > daxctl/list.c | 1 + > 2 files changed, 8 insertions(+) > > diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c > index 9e503201..0552f6d7 100644 > --- a/daxctl/lib/libdaxctl.c > +++ b/daxctl/lib/libdaxctl.c > @@ -119,11 +119,18 @@ DAXCTL_EXPORT struct daxctl_ctx *daxctl_ref(struct daxctl_ctx *ctx) > */ > DAXCTL_EXPORT void daxctl_unref(struct daxctl_ctx *ctx) > { > + struct daxctl_region *region; > + > if (ctx == NULL) > return; > ctx->refcount--; > if (ctx->refcount > 0) > return; > + > + while ((region = list_top(&ctx->regions, struct daxctl_region, list)) != > + NULL) > + daxctl_region_unref(region); > + > info(ctx, "context %p released\n", ctx); > free(ctx); > } > diff --git a/daxctl/list.c b/daxctl/list.c > index 254f0ac9..9b18ae8d 100644 > --- a/daxctl/list.c > +++ b/daxctl/list.c > @@ -134,6 +134,7 @@ int cmd_list(int argc, const char **argv, void *ctx) > else if (jdevs) > util_display_json_array(stdout, jdevs, jflag); > > + daxctl_unref(ctx); This should move to daxctl/daxctl.c similar to the ndctl_unref() in ndctl/ndctl.c
On 04/09/2018 07:39 PM, Dan Williams wrote: > On Mon, Apr 9, 2018 at 5:13 PM, Dave Jiang <dave.jiang@intel.com> wrote: >> daxctl list is not calling daxctl_unref() when executed succesfully. At the same >> time, daxctl_region_unref() is not being called when daxctl_unref() executes. >> Valgrind is reporting unfreed memory. Adding the appropriate calls to make sure >> all memory allocated are freed for daxctl list. >> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >> --- >> daxctl/lib/libdaxctl.c | 7 +++++++ >> daxctl/list.c | 1 + >> 2 files changed, 8 insertions(+) >> >> diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c >> index 9e503201..0552f6d7 100644 >> --- a/daxctl/lib/libdaxctl.c >> +++ b/daxctl/lib/libdaxctl.c >> @@ -119,11 +119,18 @@ DAXCTL_EXPORT struct daxctl_ctx *daxctl_ref(struct daxctl_ctx *ctx) >> */ >> DAXCTL_EXPORT void daxctl_unref(struct daxctl_ctx *ctx) >> { >> + struct daxctl_region *region; >> + >> if (ctx == NULL) >> return; >> ctx->refcount--; >> if (ctx->refcount > 0) >> return; >> + >> + while ((region = list_top(&ctx->regions, struct daxctl_region, list)) != >> + NULL) >> + daxctl_region_unref(region); >> + >> info(ctx, "context %p released\n", ctx); >> free(ctx); >> } >> diff --git a/daxctl/list.c b/daxctl/list.c >> index 254f0ac9..9b18ae8d 100644 >> --- a/daxctl/list.c >> +++ b/daxctl/list.c >> @@ -134,6 +134,7 @@ int cmd_list(int argc, const char **argv, void *ctx) >> else if (jdevs) >> util_display_json_array(stdout, jdevs, jflag); >> >> + daxctl_unref(ctx); > > This should move to daxctl/daxctl.c similar to the ndctl_unref() in > ndctl/ndctl.c > With the way things are coded right now that's the exit function and we can't have it in daxctl.c. main_handle_internal_commands gets called and that invokes exit() on the matched commands. We never return from that.
On Tue, Apr 10, 2018 at 8:59 AM, Dave Jiang <dave.jiang@intel.com> wrote: > > > On 04/09/2018 07:39 PM, Dan Williams wrote: >> On Mon, Apr 9, 2018 at 5:13 PM, Dave Jiang <dave.jiang@intel.com> wrote: >>> daxctl list is not calling daxctl_unref() when executed succesfully. At the same >>> time, daxctl_region_unref() is not being called when daxctl_unref() executes. >>> Valgrind is reporting unfreed memory. Adding the appropriate calls to make sure >>> all memory allocated are freed for daxctl list. >>> >>> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >>> --- >>> daxctl/lib/libdaxctl.c | 7 +++++++ >>> daxctl/list.c | 1 + >>> 2 files changed, 8 insertions(+) >>> >>> diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c >>> index 9e503201..0552f6d7 100644 >>> --- a/daxctl/lib/libdaxctl.c >>> +++ b/daxctl/lib/libdaxctl.c >>> @@ -119,11 +119,18 @@ DAXCTL_EXPORT struct daxctl_ctx *daxctl_ref(struct daxctl_ctx *ctx) >>> */ >>> DAXCTL_EXPORT void daxctl_unref(struct daxctl_ctx *ctx) >>> { >>> + struct daxctl_region *region; >>> + >>> if (ctx == NULL) >>> return; >>> ctx->refcount--; >>> if (ctx->refcount > 0) >>> return; >>> + >>> + while ((region = list_top(&ctx->regions, struct daxctl_region, list)) != >>> + NULL) >>> + daxctl_region_unref(region); >>> + >>> info(ctx, "context %p released\n", ctx); >>> free(ctx); >>> } >>> diff --git a/daxctl/list.c b/daxctl/list.c >>> index 254f0ac9..9b18ae8d 100644 >>> --- a/daxctl/list.c >>> +++ b/daxctl/list.c >>> @@ -134,6 +134,7 @@ int cmd_list(int argc, const char **argv, void *ctx) >>> else if (jdevs) >>> util_display_json_array(stdout, jdevs, jflag); >>> >>> + daxctl_unref(ctx); >> >> This should move to daxctl/daxctl.c similar to the ndctl_unref() in >> ndctl/ndctl.c >> > With the way things are coded right now that's the exit function and we > can't have it in daxctl.c. main_handle_internal_commands gets called and > that invokes exit() on the matched commands. We never return from that. Then we need to fix that. cmd_list() should not be releasing references that it did not take. How is daxctl::cmd_list() leaky but ndctl::cmd_list() is not?
diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c index 9e503201..0552f6d7 100644 --- a/daxctl/lib/libdaxctl.c +++ b/daxctl/lib/libdaxctl.c @@ -119,11 +119,18 @@ DAXCTL_EXPORT struct daxctl_ctx *daxctl_ref(struct daxctl_ctx *ctx) */ DAXCTL_EXPORT void daxctl_unref(struct daxctl_ctx *ctx) { + struct daxctl_region *region; + if (ctx == NULL) return; ctx->refcount--; if (ctx->refcount > 0) return; + + while ((region = list_top(&ctx->regions, struct daxctl_region, list)) != + NULL) + daxctl_region_unref(region); + info(ctx, "context %p released\n", ctx); free(ctx); } diff --git a/daxctl/list.c b/daxctl/list.c index 254f0ac9..9b18ae8d 100644 --- a/daxctl/list.c +++ b/daxctl/list.c @@ -134,6 +134,7 @@ int cmd_list(int argc, const char **argv, void *ctx) else if (jdevs) util_display_json_array(stdout, jdevs, jflag); + daxctl_unref(ctx); if (did_fail) return -ENOMEM; return 0;
daxctl list is not calling daxctl_unref() when executed succesfully. At the same time, daxctl_region_unref() is not being called when daxctl_unref() executes. Valgrind is reporting unfreed memory. Adding the appropriate calls to make sure all memory allocated are freed for daxctl list. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- daxctl/lib/libdaxctl.c | 7 +++++++ daxctl/list.c | 1 + 2 files changed, 8 insertions(+)