On Thu, Dec 04, 2014 at 09:40:10AM +0100, Philipp Zabel wrote: > You are right, no I don't want this. When I initially wrote this patch I > was under the impression that the memory allocated by devm_kzalloc in > bind() wouldn't be freed on unbind(). Resources claimed inside bind() will be freed in unbind(). Resources claimed in the driver's probe() will be freed in driver's remove(). They nest, and each is properly dealt with at the appropriate time due to... > I remember I stopped pursuing this > patch when I got aware of the devres_open/close/remove_group dance in > the component framework code, but somehow forgot to drop it altogether > locally. ... the use of devres grouping. So, if you use devm_kzalloc() in the driver's probe() function, then that memory will be freed after the driver's remove() function is called. That's fine. The point I was making is: probe() mem = devm_kzalloc(); mem->mmio = ...; ... bind() mem->mmio is set other members of mem are zero ... unbind() ... bind() mem->mmio is set other members of mem are indeterminant ... unbind() ... remove() mem will be freed automatically That's where the problem happens - the second time the bind() function gets called: you might as well not use devm_k*z*alloc() initially, because having the structure initialised to zero _could_ very well hide bugs. When you consider that it's not just the driver code which you have to audit, but also any code the driver calls (eg, because you've embedded some subsystem's struct in your driver private data) it quickly becomes very easy for a bug to creep in here. If we want to go down the route of having the probe function deal with resources etc, then I would recommend against using devm_kzalloc() to allocate the private structure: use devm_kmalloc() instead, which will leave the memory uninitialised. That means you get the same condition on each bind(), which means you have to think more about how to ensure that the initial state of members is dealt with throughout your driver. Alternatively, separate the struct into two sections: one which contains everything initialised by the probe() function, and everything else, and arrange for everything else to get memset() on entry to bind(). -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
