Re: [PATCH] i2c: i801: fix memleak on probe error

[Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>]

Hi Peter,

Joining the discussion late as I was on vacation...

On Mon, 23 Dec 2013 11:43:21 +0100, Peter Wu wrote:
> Nevermind this patch, it does not really fix the memleak because
> i2c_set_adapdata() calls dev_set_drvdata() which allocates memory.
> (I must have ran kmemleak too early, right after boot it did not
> give any warnings, now it does).

I can confirm that your proposed patch doesn't fix anything. It makes
no difference if pci_set_drvdata(dev, priv) is called earlier or later.

> RFC: what about dropping i2c_set_adapdata() from the probe function
> and replacing i2c_get_adapdata(adapter) by
> pci_get_drvdata(adapter->pci_dev) on top of this patch? I am not
> sure what the purpose is for i2c_set_adapdata, hence this question.

The purpose of i2c_set_adapdata is to attach a data structure to a
specific i2c_adapter device. In the case of the i2c-i801 driver,
there's only one such (class) device per PCI device so we could indeed
reuse the PCI device data pointer as you suggest above. But in the
general case there can be several i2c_adapter devices and each needs
its own data.

Also for performance reasons we don't want to do that because
pci_get_drvdata(adapter->pci_dev) is slower than
i2c_get_adapdata(adapter) (due to the extra pointer deference.) As this
happens repeatedly at run-time (in function i801_access) we want it to
be as fast as possible.

Note that we could (ab)use i2c_adapter.algo_data to store the
i2c_adapter device-specific data. Some drivers are doing exactly that,
for example i2c-nforce2. I suspect this legacy field is now somewhat
redundant with i2c_set_adapdata() as I couldn't find any i2c bus
driver using both.

-- 
Jean Delvare