On Sun, Apr 28, 2013 at 12:47 PM, Mark Brown broonie@kernel.org wrote:
On Sun, Apr 28, 2013 at 08:00:29AM +0300, Leon Romanovsky wrote:
On Thu, Apr 25, 2013 at 3:52 PM, Mark Brown broonie@kernel.org wrote:
This seems like a bug that should be fixed on the API side, it's really not what you'd expect a devm function to do, and if you are explicitly freeing a devm allocated object there's an expectation that you need to call a corresponding devm cleanup function.
I can't agree with you about the "expectation" - device memory can be in use. This code will free memory after all references will be freed. Additionally it is up-to developer to decide what to do if input_register_device call failed. According to the code base (http://lxr.free-electrons.com/ident?i=input_free_device) the patch is not correct.
Right, but what I'm saying is that this is not how a devm_ API is expected to behave and is therefore at best error prone on two fronts - both due to the fact that you need to clean up explicitly even if the resource is not actually managed and also due to the fact that the regular free function is being used rather than a devm_ one when explicit deallocation is done. Neither of these things is normal for a devm_ API. The behaviour you're describing says that the managed function should actually be registration not allocation.
I think the reason of our misunderstanding is due to the name of input_free_device call. From the code, it is device destroy function, and the freeing itself done as an error handling of input_register_device (http://lxr.free-electrons.com/source/drivers/input/input.c#L2114).
How do you think we need to proceed? Do I need to send patches with explicit call to input_free_device function?
-- Leon Romanovsky | Independent Linux Consultant www.leon.nu | leon@leon.nu