[U-Boot] [PATCH v2 00/14] Devres (Managed Device Resource) for U-Boot

[Masahiro Yamada]

Hi Albert,


2015-07-13 15:51 GMT+09:00 Albert ARIBAUD <albert.u.boot at aribaud.net>:
> Hello Masahiro,
>
> On Mon, 13 Jul 2015 13:17:03 +0900, Masahiro Yamada
> <yamada.masahiro at socionext.com> wrote:
>>
>> Please refer to the commit message of 06/14
>> for what this series wants to do.
>
> Remark: you could use "Series-notes:" in 6/14 to have patman directly
> include said notes here instead of referring the reader to the patch.
>
>> Masahiro Yamada (14):
>>   x86: delete unneeded declarations of disable_irq() and enable_irq()
>>   linux_compat: remove cpu_relax() define
>>   linux_compat: move vzalloc() to header file as an inline function
>>   linux_compat: handle __GFP_ZERO in kmalloc()
>>   dm: add DM_FLAG_BOUND flag
>>   devres: introduce Devres (Managed Device Resource) framework
>>   devres: add devm_kmalloc() and friends (managed memory allocators)
>>   dm: refactor device_bind() and device_unbind() with devm_kzalloc()
>>   dm: merge fail_alloc labels
>>   linux_compat: introduce GFP_DMA flag for kmalloc()
>>   dm: refactor device_probe() and device_remove() with devm_kzalloc()
>>   devres: add debug command to dump device resources
>>   dm: remove redundant CONFIG_DM from driver/core/Makefile
>>   devres: compile Devres iif CONFIG_DM_DEVICE_REMOVE is enabled
>
> I am still unsure why this is necessary. I had a discussion on the
> list with Simon, see last message here:
>
> <https://www.mail-archive.com/u-boot@lists.denx.de/msg177031.html>
>
> Unless I'm mistaken, the only case where we actually have a leak that
> this series would fix is in some cases of binding USB devices / drivers
> multiple times, and even then, it would take a considerable amount of
> repeated bindings before U-Boot could become unable to boot a payload;
> a scenario that I find unlikely.
>
> I do understand the general goal of fixing bugs when we ind them; but I
> question the global benefit of fixing this specific leak bug by adding a
> whole new feature with a lot of code to U-Boot, as opposed to fixing
> it in a more ad hoc way with much less less code volume and complexity.


You have a point.

What we really want to avoid is to make low-level drivers too complicated
by leaving the correct memory management to each of them.

After all, there turned out to be two options to solve it.

  [1] Simon's driver model:  move allocating/freeing memory to the driver core
                             by having only the needed memory size
specified in each driver
  [2] Devres: we still have to allocate memory in each driver, but we
need not free it explicitly,
               making our driver work much easier


[1] and [2] are completely differently approach,
but what they solve is the same:  easier memory (resource) management
without leak.

The only problem I see in [1] is that it is not controllable at run-time.
The memory size for the auto-allocation must be specified at the compile time.

So, we need calloc() and free() in some low-level drivers.
Simon might say they are only a few "exceptions",
(my impression is I often hear the logic such as "it is not a problem
because we do not have many.")
Anyway, we had already lost the consistency as for memory allocation.

I imagined if [2] had been supported earlier, we would not have needed [1].
(at least, [2] seems more flexible to me.)


We already have many DM-based drivers, and I think we can live with [1]
despite some exceptional drivers allocating memory on their own.

So, if Simon (and other developers) does not feel comfortable with this series,
I do not mind discarding it.


-- 
Best Regards
Masahiro Yamada