Re: [PATCH] media: i2c: maxim-serdes: reduce max_des_update_streams() stack usage

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Arnd,

On Thu, Oct 15, 2026 at 08:45:26AM +0200, Arnd Bergmann wrote:
> On Wed, Oct 14, 2026, at 13:33, Sakari Ailus wrote:
> > On Wed, Oct 14, 2026 at 12:31:33PM +0200, Arnd Bergmann wrote:
> >>  			return ret;
> >> @@ -2450,18 +2453,21 @@ static int max_des_update_streams(struct v4l2_subdev *sd,
> >>  				  u32 pad, u64 updated_streams_mask, bool enable)
> >>  {
> >>  	struct max_des_priv *priv = v4l2_get_subdevdata(sd);
> >> -	struct max_des_remap_context context = { 0 };
> >> +	struct max_des_remap_context *context __free(kfree) = kzalloc_obj(*context);
> >
> > max_des_update_streams() is used in implementing disable_streams()
> > sub-device pad op where meaningful error handling isn't possible.
> >
> > It seems there are other ways it can fail but this adds one more.
> 
> Right. I see there are more allocations in max_des_update_pipe_remaps(),
> and max_des_update_pipe_vc_remaps() called through max_des_update_links()
> that could fail as well, and may have the additional problem of
> leaking allocations every time a stream is enabled and disabled.
> 
> Can you tell if those should be fixed as well?

There seem to be devm_*alloc*() functions being used but that doesn't look
like a memory leak -- there's devm_kfree() use as well for the same
pointers.

> 
> > The bulk of the struct consists of the vc_ids_map unsigned int array, but
> > the maximum value VCs can have is 31. Also the size of unsigned long
> > depends on the architecture, so a fixed-size type would be probably a
> > better choice.
> >
> > Also using bit fields instead of bool would save space.
> 
> That would likely bring it under the warning limit, but I wonder
> if there is another way to avoid the stack allocation entirely.
> 
> Are ->enable_streams() and ->disable_streams() always serialized
> or can they be called concurrently on the same subdev? If there
> is only one of them at a time, the allocation could just be
> done ahead of time in max_des_allocate().

Access to sub-device state which these ops operate on is serialised using
the sub-device state mutex i.e. these aren't called concurrently (on the
same sub-device).

Also cc Dumitru.

-- 
Regards,

Sakari Ailus