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

From: Arnd Bergmann

Date: Thu Oct 15 2026 - 02:46:20 EST


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?

> 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().

Arnd