lkml.org 
[lkml]   [2021]   [Dec]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 16/23] counter: interrupt-cnt: Convert to new counter registration
From
Date
On 12/27/21 11:59 AM, Greg Kroah-Hartman wrote:
> On Mon, Dec 27, 2021 at 10:45:19AM +0100, Uwe Kleine-König wrote:
>> This fixes device lifetime issues where it was possible to free a live
>> struct device.
>>
>> Fixes: a55ebd47f21f ("counter: add IRQ or GPIO based counter")
>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> ---
>> drivers/counter/interrupt-cnt.c | 28 ++++++++++++++++------------
>> 1 file changed, 16 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/counter/interrupt-cnt.c b/drivers/counter/interrupt-cnt.c
>> index 4bf706ef46e2..9e99702470c2 100644
>> --- a/drivers/counter/interrupt-cnt.c
>> +++ b/drivers/counter/interrupt-cnt.c
>> @@ -16,7 +16,6 @@
>>
>> struct interrupt_cnt_priv {
>> atomic_t count;
>> - struct counter_device counter;
>> struct gpio_desc *gpio;
>> int irq;
>> bool enabled;
>> @@ -148,12 +147,14 @@ static const struct counter_ops interrupt_cnt_ops = {
>> static int interrupt_cnt_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> + struct counter_device *counter;
>> struct interrupt_cnt_priv *priv;
>> int ret;
>>
>> - priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> - if (!priv)
>> + counter = devm_counter_alloc(dev, sizeof(*priv));
> I just picked one of these patches at random, nothing specific about
> this driver...
>
> You can not have a 'struct device' in memory allocated by devm_*()
> functions for the obvious reason that now that memory is being
> controlled by a reference count that is OUTSIDE of the structure itself.
>
> So while your goal might be good here, this is not the correct solution
> at all, sorry.

Before this patch the memory for the struct device was devm_kzalloc'ed.
Which as you point out is a bug.

After this patch the memory is reference counted and will be freed when
the last reference is dropped, in the release callback of the struct device.

The alloc function is still a devm_ function, but on 'free' it will only
drop the reference to the struct device that it holds. This is a very
common pattern that is used by basically any driver subsystem in the kernel.

This is the correct solution to the problem.

\
 
 \ /
  Last update: 2021-12-27 12:22    [W:0.315 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site