On 04/10/2018 10:38 AM, Plewa, Lukasz wrote: > On Tue, Apr 10, 2018 at 7:17 PM, Dave Jiang <[email protected]> wrote: >> When daxctl_unref is releasing the context, we should make sure that the >> regions and devices are also being released. free_region() will free all the >> devices under the region. >> >> Signed-off-by: Dave Jiang <[email protected]> >> --- >> >> v2: Use list_for_each_safe() for region removal. (Dan) >> >> daxctl/lib/libdaxctl.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c index >> 9e503201..22f4210a 100644 >> --- a/daxctl/lib/libdaxctl.c >> +++ b/daxctl/lib/libdaxctl.c >> @@ -29,6 +29,8 @@ >> >> static const char *attrs = "dax_region"; >> >> +static void free_region(struct daxctl_region *region, struct list_head >> +*head); >> + >> /** >> * struct daxctl_ctx - library user context to find "nd" instances >> * >> @@ -119,11 +121,17 @@ DAXCTL_EXPORT struct daxctl_ctx >> *daxctl_ref(struct daxctl_ctx *ctx) >> */ >> DAXCTL_EXPORT void daxctl_unref(struct daxctl_ctx *ctx) { >> + struct daxctl_region *region, *_r; >> + >> if (ctx == NULL) >> return; >> ctx->refcount--; >> if (ctx->refcount > 0) >> return; >> + >> + list_for_each_safe(&ctx->regions, region, _r, list) >> + free_region(region, &ctx->regions); >> + >> info(ctx, "context %p released\n", ctx); >> free(ctx); >> } > > As daxctl_region has its own refcount, you need daxctl_ref() in > add_dax_region() and daxctl_unref() in free_region().( or daxctl_ref() in > daxctl_region_ref() and daxctl_unref() in daxctl_region_unref()) > > Without it, this code will cause segfault: > > daxctl_new(&ctx); > region = daxctl_new_region(...); > daxctl_region_ref(region); > daxctl_unref(ctx); > daxctl_region_unref(region);
Shouldn't it go in this order: daxctl_region_unref(region); daxctl_unref(ctx); In this case, you won't segfault right? I think ctx should be the very last thing you free. > > Łukasz > _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
