RE: [PATCH] usb: phy: tegra: Using devm API for memory allocation

2012-12-20 Thread Venu Byravarasu
> -Original Message-
> From: Stephen Warren [mailto:swar...@wwwdotorg.org]
> Sent: Wednesday, December 19, 2012 11:08 PM
> To: Venu Byravarasu
> Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org; linux-...@vger.kernel.org
> Subject: Re: [PATCH] usb: phy: tegra: Using devm API for memory allocation
> 
> On 12/18/2012 10:38 PM, Venu Byravarasu wrote:
> >> -Original Message-
> >> From: Stephen Warren [mailto:swar...@wwwdotorg.org]
> >> Sent: Tuesday, December 18, 2012 10:03 PM
> >> To: Venu Byravarasu
> >> Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-
> >> ker...@vger.kernel.org; linux-...@vger.kernel.org
> >> Subject: Re: [PATCH] usb: phy: tegra: Using devm API for memory
> allocation
> >>
> >
> > Stephen,
> > As you mentioned I can replace kmalloc with kzalloc in the original code
> > and push an updated patch.
> > However, I just wanted to understand if there exists any issue
> > in using devm_kzalloc instead of kzalloc?
> 
> devm_* are intended for objects allocated during probe(), and free()d
> during remove(). The object you're allocating here isn't that case.
> 
> Now, once you convert the Tegra PHY driver to be a true device, perhaps
> this object will be allocated/freed during probe/remove, so the devm_
> functions will be useful then?
> 
> The problem this may cause is a memory leak. Consider the Tegra EHCI and
> PHY drivers being built as modules, the PHY driver module being inserted
> and never removed, yet the EHCI driver being continually inserted and
> removed. Since the PHY is never removed, the memory allocated by its
> devm_kzalloc() call is never freed, but it's continually re-allocated
> since tegra_usb_phy_open() is called whenever the EHCI driver module is
> inserted. You need the explicit kfree() to avoid that, and since you're
> kfree()ing somewhere other than remove(), using devm_* to make the
> allocation isn't appropriate.
 
Thanks Stephen for the detailed explanation.
Sent updated patch for review: 
http://marc.info/?l=linux-usb=135599303216132=2 .

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] usb: phy: tegra: Using devm API for memory allocation

2012-12-20 Thread Venu Byravarasu
 -Original Message-
 From: Stephen Warren [mailto:swar...@wwwdotorg.org]
 Sent: Wednesday, December 19, 2012 11:08 PM
 To: Venu Byravarasu
 Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-
 ker...@vger.kernel.org; linux-...@vger.kernel.org
 Subject: Re: [PATCH] usb: phy: tegra: Using devm API for memory allocation
 
 On 12/18/2012 10:38 PM, Venu Byravarasu wrote:
  -Original Message-
  From: Stephen Warren [mailto:swar...@wwwdotorg.org]
  Sent: Tuesday, December 18, 2012 10:03 PM
  To: Venu Byravarasu
  Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-
  ker...@vger.kernel.org; linux-...@vger.kernel.org
  Subject: Re: [PATCH] usb: phy: tegra: Using devm API for memory
 allocation
 
 
  Stephen,
  As you mentioned I can replace kmalloc with kzalloc in the original code
  and push an updated patch.
  However, I just wanted to understand if there exists any issue
  in using devm_kzalloc instead of kzalloc?
 
 devm_* are intended for objects allocated during probe(), and free()d
 during remove(). The object you're allocating here isn't that case.
 
 Now, once you convert the Tegra PHY driver to be a true device, perhaps
 this object will be allocated/freed during probe/remove, so the devm_
 functions will be useful then?
 
 The problem this may cause is a memory leak. Consider the Tegra EHCI and
 PHY drivers being built as modules, the PHY driver module being inserted
 and never removed, yet the EHCI driver being continually inserted and
 removed. Since the PHY is never removed, the memory allocated by its
 devm_kzalloc() call is never freed, but it's continually re-allocated
 since tegra_usb_phy_open() is called whenever the EHCI driver module is
 inserted. You need the explicit kfree() to avoid that, and since you're
 kfree()ing somewhere other than remove(), using devm_* to make the
 allocation isn't appropriate.
 
Thanks Stephen for the detailed explanation.
Sent updated patch for review: 
http://marc.info/?l=linux-usbm=135599303216132w=2 .

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] usb: phy: tegra: Using devm API for memory allocation

2012-12-19 Thread Stephen Warren
On 12/18/2012 10:38 PM, Venu Byravarasu wrote:
>> -Original Message-
>> From: Stephen Warren [mailto:swar...@wwwdotorg.org]
>> Sent: Tuesday, December 18, 2012 10:03 PM
>> To: Venu Byravarasu
>> Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-
>> ker...@vger.kernel.org; linux-...@vger.kernel.org
>> Subject: Re: [PATCH] usb: phy: tegra: Using devm API for memory allocation
>>
>> On 12/17/2012 11:21 PM, Venu Byravarasu wrote:
>>> Using devm_kzalloc for allocating memory needed for PHY
>>> pointer and hence removing kfree calls to PHY pointer.
>>
>> Since the kfree() here used to be in tegra_usb_phy_close() rather than
>> any remove() function, does it actually make sense to use
>> devm_kzalloc(); would plain using kzalloc() instead, and not removing
>> the kfree() calls, be better?
>>
>  
> Stephen,
> As you mentioned I can replace kmalloc with kzalloc in the original code 
> and push an updated patch.
> However, I just wanted to understand if there exists any issue
> in using devm_kzalloc instead of kzalloc?

devm_* are intended for objects allocated during probe(), and free()d
during remove(). The object you're allocating here isn't that case.

Now, once you convert the Tegra PHY driver to be a true device, perhaps
this object will be allocated/freed during probe/remove, so the devm_
functions will be useful then?

The problem this may cause is a memory leak. Consider the Tegra EHCI and
PHY drivers being built as modules, the PHY driver module being inserted
and never removed, yet the EHCI driver being continually inserted and
removed. Since the PHY is never removed, the memory allocated by its
devm_kzalloc() call is never freed, but it's continually re-allocated
since tegra_usb_phy_open() is called whenever the EHCI driver module is
inserted. You need the explicit kfree() to avoid that, and since you're
kfree()ing somewhere other than remove(), using devm_* to make the
allocation isn't appropriate.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] usb: phy: tegra: Using devm API for memory allocation

2012-12-19 Thread Stephen Warren
On 12/18/2012 10:38 PM, Venu Byravarasu wrote:
 -Original Message-
 From: Stephen Warren [mailto:swar...@wwwdotorg.org]
 Sent: Tuesday, December 18, 2012 10:03 PM
 To: Venu Byravarasu
 Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-
 ker...@vger.kernel.org; linux-...@vger.kernel.org
 Subject: Re: [PATCH] usb: phy: tegra: Using devm API for memory allocation

 On 12/17/2012 11:21 PM, Venu Byravarasu wrote:
 Using devm_kzalloc for allocating memory needed for PHY
 pointer and hence removing kfree calls to PHY pointer.

 Since the kfree() here used to be in tegra_usb_phy_close() rather than
 any remove() function, does it actually make sense to use
 devm_kzalloc(); would plain using kzalloc() instead, and not removing
 the kfree() calls, be better?

  
 Stephen,
 As you mentioned I can replace kmalloc with kzalloc in the original code 
 and push an updated patch.
 However, I just wanted to understand if there exists any issue
 in using devm_kzalloc instead of kzalloc?

devm_* are intended for objects allocated during probe(), and free()d
during remove(). The object you're allocating here isn't that case.

Now, once you convert the Tegra PHY driver to be a true device, perhaps
this object will be allocated/freed during probe/remove, so the devm_
functions will be useful then?

The problem this may cause is a memory leak. Consider the Tegra EHCI and
PHY drivers being built as modules, the PHY driver module being inserted
and never removed, yet the EHCI driver being continually inserted and
removed. Since the PHY is never removed, the memory allocated by its
devm_kzalloc() call is never freed, but it's continually re-allocated
since tegra_usb_phy_open() is called whenever the EHCI driver module is
inserted. You need the explicit kfree() to avoid that, and since you're
kfree()ing somewhere other than remove(), using devm_* to make the
allocation isn't appropriate.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] usb: phy: tegra: Using devm API for memory allocation

2012-12-18 Thread Venu Byravarasu
> -Original Message-
> From: Stephen Warren [mailto:swar...@wwwdotorg.org]
> Sent: Tuesday, December 18, 2012 10:03 PM
> To: Venu Byravarasu
> Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org; linux-...@vger.kernel.org
> Subject: Re: [PATCH] usb: phy: tegra: Using devm API for memory allocation
> 
> On 12/17/2012 11:21 PM, Venu Byravarasu wrote:
> > Using devm_kzalloc for allocating memory needed for PHY
> > pointer and hence removing kfree calls to PHY pointer.
> 
> Since the kfree() here used to be in tegra_usb_phy_close() rather than
> any remove() function, does it actually make sense to use
> devm_kzalloc(); would plain using kzalloc() instead, and not removing
> the kfree() calls, be better?
> 
 
Stephen,
As you mentioned I can replace kmalloc with kzalloc in the original code 
and push an updated patch.
However, I just wanted to understand if there exists any issue
in using devm_kzalloc instead of kzalloc?

> When the PHY code gets converted to be an actual probed driver, then
> perhaps using devm will make sense.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] usb: phy: tegra: Using devm API for memory allocation

2012-12-18 Thread Stephen Warren
On 12/17/2012 11:21 PM, Venu Byravarasu wrote:
> Using devm_kzalloc for allocating memory needed for PHY
> pointer and hence removing kfree calls to PHY pointer.

Since the kfree() here used to be in tegra_usb_phy_close() rather than
any remove() function, does it actually make sense to use
devm_kzalloc(); would plain using kzalloc() instead, and not removing
the kfree() calls, be better?

When the PHY code gets converted to be an actual probed driver, then
perhaps using devm will make sense.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] usb: phy: tegra: Using devm API for memory allocation

2012-12-18 Thread Stephen Warren
On 12/17/2012 11:21 PM, Venu Byravarasu wrote:
 Using devm_kzalloc for allocating memory needed for PHY
 pointer and hence removing kfree calls to PHY pointer.

Since the kfree() here used to be in tegra_usb_phy_close() rather than
any remove() function, does it actually make sense to use
devm_kzalloc(); would plain using kzalloc() instead, and not removing
the kfree() calls, be better?

When the PHY code gets converted to be an actual probed driver, then
perhaps using devm will make sense.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] usb: phy: tegra: Using devm API for memory allocation

2012-12-18 Thread Venu Byravarasu
 -Original Message-
 From: Stephen Warren [mailto:swar...@wwwdotorg.org]
 Sent: Tuesday, December 18, 2012 10:03 PM
 To: Venu Byravarasu
 Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-
 ker...@vger.kernel.org; linux-...@vger.kernel.org
 Subject: Re: [PATCH] usb: phy: tegra: Using devm API for memory allocation
 
 On 12/17/2012 11:21 PM, Venu Byravarasu wrote:
  Using devm_kzalloc for allocating memory needed for PHY
  pointer and hence removing kfree calls to PHY pointer.
 
 Since the kfree() here used to be in tegra_usb_phy_close() rather than
 any remove() function, does it actually make sense to use
 devm_kzalloc(); would plain using kzalloc() instead, and not removing
 the kfree() calls, be better?
 
 
Stephen,
As you mentioned I can replace kmalloc with kzalloc in the original code 
and push an updated patch.
However, I just wanted to understand if there exists any issue
in using devm_kzalloc instead of kzalloc?

 When the PHY code gets converted to be an actual probed driver, then
 perhaps using devm will make sense.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/