Re: [PATCH v7 3/7] drm/i915: Prepare for multiple GTs

[Andi Shyti <andi.shyti@linux.intel.com>]

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