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

["Arnd Bergmann" <arnd@arndb.de>]

On Thu, Oct 15, 2026, at 11:40, Sakari Ailus wrote:
> 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:
>> 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.

I see. Yes, that is a bit unusual but not a bug.

Seeing that the max_serdes_vc_remap array is only eight bytes long, the
devm_kzalloc() does seem like a very heavyweight operation here, that one
can just go on the stack and into struct max_des_pipe in place of
the pointer ;-)

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

Ok, so maybe the easiest way would be to just put everything into 
max_des_priv, including all the currently dynamically allocated
arrays. Since all the maximum sizes are 4 or 8 in the driver
and most of the structures are short, using fixed-length arrays
will make it use less memory than the minimum of two cache lines
per devres object.

      Arnd