Hi Michal,

[...]

> > +static int intel_gt_tile_setup(struct intel_gt *gt, phys_addr_t phys_addr)
> > +{
> > +   int ret;
> > +
> > +   if (!gt_is_root(gt)) {
> > +           struct intel_uncore_mmio_debug *mmio_debug;
> > +           struct intel_uncore *uncore;
> > +
> > +           uncore = kzalloc(sizeof(*uncore), GFP_KERNEL);
> > +           if (!uncore)
> > +                   return -ENOMEM;
> > +
> > +           mmio_debug = kzalloc(sizeof(*mmio_debug), GFP_KERNEL);
> > +           if (!mmio_debug) {
> > +                   kfree(uncore);
> > +                   return -ENOMEM;
> > +           }
> > +
> > +           gt->uncore = uncore;
> > +           gt->uncore->debug = mmio_debug;
> > +
> > +           __intel_gt_init_early(gt);
> > +   }
> > +
> > +   intel_uncore_init_early(gt->uncore, gt);
> > +
> > +   ret = intel_uncore_setup_mmio(gt->uncore, phys_addr);
> > +   if (ret)
> > +           return ret;
> 
> (little guessing as in this patch we don't have non-root gt yet)
> 
> if the future, when we will be doing setup of non-root gt, if we return
> here then likely we will leak both uncore/mmio_debug as gt will not be
> added to i915->gts thus it will not be visible in for_each_gt loop used
> to release/cleanup all gts.
> 
> since in above code you are doing cleanup in case of kzalloc failure,
> same should be done in case of mmio setup failure.

that's a good point. In the next patch I am going to add support
for the first multitile platform and, because it's too old to
remember, I had a look and I think this part is not properly
managed.

Thanks for the note!

Andi

Reply via email to