Le 15/12/2020 à 20:35, Dan Carpenter a écrit :
On Tue, Dec 15, 2020 at 08:08:15PM +0100, Maxime Ripard wrote:
On Tue, Dec 15, 2020 at 07:18:48PM +0100, Christophe JAILLET wrote:
Le 15/12/2020 à 12:37, Maxime Ripard a écrit :
On Tue, Dec 15, 2020 at 12:11:53PM +0300, Dan Carpenter wrote:
On Tue, Dec 15, 2020 at 09:56:55AM +0100, Maxime Ripard wrote:
Hi,
On Mon, Dec 14, 2020 at 09:21:17PM +0100, Christophe JAILLET wrote:
'irq_of_parse_and_map()' should be balanced by a corresponding
'irq_dispose_mapping()' call. Otherwise, there is some resources leaks.
Do you have a source to back that? It's not clear at all from the
documentation for those functions, and couldn't find any user calling it
from the ten-or-so random picks I took.
It looks like irq_create_of_mapping() needs to be freed with
irq_dispose_mapping() so this is correct.
The doc should be updated first to make that clear then, otherwise we're
going to fix one user while multiples will have poped up
Maxime
Hi,
as Dan explained, I think that 'irq_dispose_mapping()' is needed because of
the 'irq_create_of_mapping()" within 'irq_of_parse_and_map()'.
As you suggest, I'll propose a doc update to make it clear and more future
proof.
Thanks :)
And if you feel like it, a coccinelle script would be awesome too so
that other users get fixed over time
Maxime
Smatch has a new check for resource leaks which hopefully people will
find useful.
https://github.com/error27/smatch/blob/master/check_unwind.c
Nice :)
I wasn't aware of it.
To check for these I would need to add the following lines to the table:
{ "irq_of_parse_and_map", ALLOC, -1, "$", &int_one, &int_max},
{ "irq_create_of_mapping", ALLOC, -1, "$", &int_one, &int_max},
{ "irq_dispose_mapping", RELEASE, 0, "$"},
The '-1, "$"' means the returned value. irq_of_parse_and_map() and
irq_create_of_mapping() return positive int on success.
The irq_dispose_mapping() frees its zeroth parameter so it's listed as
'0, "$"'. We don't care about the returns from irq_dispose_mapping().
It doesn't apply in this case but if a function frees a struct member
then that's listed as '0, "$->member_name"'.
regards,
dan carpenter
The script I use to try to spot missing release function is:
///
@@
expression x, y;
identifier f, l;
@@
* x = irq_of_parse_and_map(...);
... when any
* y = f(...);
... when any
* if (<+... y ...+>)
{
...
(
* goto l;
|
* return ...;
)
...
}
... when any
*l:
... when != irq_dispose_mapping(...);
* return ...;
///
It is likely that some improvement can be made, but it works pretty well
for what I want.
And I have a collection of alloc/free functions that I manually put in
place of irq_of_parse_and_map and irq_dispose_mapping.
Up to know, this list is:
// alloc_etherdev/alloc_etherdev_mq/alloc_etherdev_mqs - free_netdev
// alloc_workqueue - destroy_workqueue
// class_register - class_unregister
// clk_get - clk_put
// clk_prepare - clk_unprepare
// create_workqueue - destroy_workqueue
// create_singlethread_workqueue - destroy_workqueue
//
dev_pm_domain_attach/dev_pm_domain_attach_by_id/dev_pm_domain_attach_by_name
- dev_pm_domain_detach
// devres_alloc - devres_free
// dma_alloc_coherent - dma_free_coherent
// dma_map_resource - dma_unmap_resource
// dma_map_single - dma_unmap_single
// dma_request_slave_channel - dma_release_channel
// dma_request_chan - dma_release_channel
// framebuffer_alloc - framebuffer_release
// get_device - put_device
// iio_channel_get - iio_channel_release
// ioremap - iounmap
// input_allocate_device - input_free_device
// input_register_handle - input_unregister_handle
// irq_of_parse_and_map / irq_create_of_mapping - irq_dispose_mapping
// kmalloc - kfree
// mempool_alloc - mempool_free
// of_node_get - of_node_put
// of_reserved_mem_device_init - of_reserved_mem_device_release
// pinctrl_register - pinctrl_unregister
// request_irq - free_irq
// request_region - release_region
// request_mem_region - release_mem_region
// reset_control_assert - reset_control_deassert
// scsi_host_alloc - scsi_host_put
// pci_alloc_irq_vectors - pci_free_irq_vectors
// pci_dev_get - pci_dev_put
// pci_enable_device - pci_disable_device
// pci_iomap - pci_iounmap
// pci_request_region - pci_release_region
// pci_request_regions - pci_release_regions
// alloc_skb/__alloc_skb - kfree_skb/__kfree_skb
// dev_alloc_skb - dev_kfree_skb
// spi_dev_get - spi_dev_put
// spi_message_alloc - spi_message_free
// spi_register_master - spi_unregister_master
