In version 1, I forgot to modify the same bug in the first loop.
we have to call kobject_put() to clean up the kobject after function
kobject_init(), kobject_add(), or kobject_uevent() is called.
Signed-off-by: Cong Ding <[email protected]>
---
drivers/uio/uio.c | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 5110f36..79774d3 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -291,10 +291,10 @@ static int uio_dev_add_attributes(struct uio_device *idev)
mem->map = map;
ret = kobject_add(&map->kobj, idev->map_dir, "map%d", mi);
if (ret)
- goto err_map;
+ goto err_map_kobj;
ret = kobject_uevent(&map->kobj, KOBJ_ADD);
if (ret)
- goto err_map;
+ goto err_map_kobj;
}
for (pi = 0; pi < MAX_UIO_PORT_REGIONS; pi++) {
@@ -317,23 +317,27 @@ static int uio_dev_add_attributes(struct uio_device *idev)
ret = kobject_add(&portio->kobj, idev->portio_dir,
"port%d", pi);
if (ret)
- goto err_portio;
+ goto err_portio_kobj;
ret = kobject_uevent(&portio->kobj, KOBJ_ADD);
if (ret)
- goto err_portio;
+ goto err_portio_kobj;
}
return 0;
err_portio:
- for (pi--; pi >= 0; pi--) {
+ pi--;
+err_portio_kobj:
+ for (; pi >= 0; pi--) {
port = &idev->info->port[pi];
portio = port->portio;
kobject_put(&portio->kobj);
}
kobject_put(idev->portio_dir);
err_map:
- for (mi--; mi>=0; mi--) {
+ mi--;
+err_map_kobj:
+ for (; mi >= 0; mi--) {
mem = &idev->info->mem[mi];
map = mem->map;
kobject_put(&map->kobj);
--
1.7.4.5
On Thu, Nov 29, 2012 at 05:40:00PM +0000, Cong Ding wrote:
> In version 1, I forgot to modify the same bug in the first loop.
>
> we have to call kobject_put() to clean up the kobject after function
> kobject_init(), kobject_add(), or kobject_uevent() is called.
Yes, thanks. Incredible how that code looks like...
There's still another bug: The memory allocated with kzalloc is
never freed. More tomorrow, have to go to sleep now.
Thanks,
Hans
>
> Signed-off-by: Cong Ding <[email protected]>
> ---
> drivers/uio/uio.c | 16 ++++++++++------
> 1 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
> index 5110f36..79774d3 100644
> --- a/drivers/uio/uio.c
> +++ b/drivers/uio/uio.c
> @@ -291,10 +291,10 @@ static int uio_dev_add_attributes(struct uio_device *idev)
> mem->map = map;
> ret = kobject_add(&map->kobj, idev->map_dir, "map%d", mi);
> if (ret)
> - goto err_map;
> + goto err_map_kobj;
> ret = kobject_uevent(&map->kobj, KOBJ_ADD);
> if (ret)
> - goto err_map;
> + goto err_map_kobj;
> }
>
> for (pi = 0; pi < MAX_UIO_PORT_REGIONS; pi++) {
> @@ -317,23 +317,27 @@ static int uio_dev_add_attributes(struct uio_device *idev)
> ret = kobject_add(&portio->kobj, idev->portio_dir,
> "port%d", pi);
> if (ret)
> - goto err_portio;
> + goto err_portio_kobj;
> ret = kobject_uevent(&portio->kobj, KOBJ_ADD);
> if (ret)
> - goto err_portio;
> + goto err_portio_kobj;
> }
>
> return 0;
>
> err_portio:
> - for (pi--; pi >= 0; pi--) {
> + pi--;
> +err_portio_kobj:
> + for (; pi >= 0; pi--) {
> port = &idev->info->port[pi];
> portio = port->portio;
> kobject_put(&portio->kobj);
> }
> kobject_put(idev->portio_dir);
> err_map:
> - for (mi--; mi>=0; mi--) {
> + mi--;
> +err_map_kobj:
> + for (; mi >= 0; mi--) {
> mem = &idev->info->mem[mi];
> map = mem->map;
> kobject_put(&map->kobj);
> --
> 1.7.4.5
>
>
Hi Hans, I think the memory allocated with kzalloc is properly freed
by calling kobject_put.
I can give a simple explanation.
1) when we call kobject_init, the parameter portio_attr_type is
passed in. portio_attr_type includes a function pointer to
portio_release, which releases the memory of portio.
2) when we call kobject_put, kref_put is called with the pointer of
function kobject_release.
3) kref_put calls kref_sub, with the same pointer of function kobject_release.
4) and kref_put calls the function kboject_release if
atomic_sub_and_test returns true
5) let's look at what kobject_release is. it calls kobject_cleanup,
and kobject_cleanup calls t->release(kobj) where t->release is exactly
the function we passed in through portio_init at step (1). so function
portio_release is called, and the memory allocated with kzalloc is
freed.
If there are anything wrong in my analysis, please feel free to let me know.
Personally, I suggest to add a function to create and release
uio_portio, which is similar as kobject_create and kobject_put in file
lib/kobject.c. In this way, it avoid other readers thinking the memory
is not freed (and we should add some comments here). For example,
uio_portio_create call kzalloc and kboject_init, and returns
uio_portio, which is similar as function kobject_create; and
uio_portio_release calls kobject_put to release the memory. And we do
same thing for uio_map.
The usage here is quite strange, but it works. If I write this
function from zero, I will use a pointer to kobject in uio_portio
struct instead of kobject struct itself. In this case I can call
kobject_create instead of kobject_init, and then we do both
kzalloc(uio_portio) and kfree(uio_portio) in the file uio.c.
Best,
Cong
On Fri, Nov 30, 2012 at 1:13 AM, Hans J. Koch <[email protected]> wrote:
> There's still another bug: The memory allocated with kzalloc is
> never freed.
On Thu, Feb 14, 2013 at 12:43:15PM +0100, Cong Ding wrote:
> On Sun, Jan 20, 2013 at 10:01:41PM +0100, Hans J. Koch wrote:
> > On Fri, Jan 18, 2013 at 02:00:50PM -0800, Greg Kroah-Hartman wrote:
> > > On Fri, Jan 18, 2013 at 10:05:45PM +0100, Cong Ding wrote:
> > > > On Tue, Dec 11, 2012 at 2:21 AM, Hans J. Koch <[email protected]> wrote:
> > > > > On Thu, Nov 29, 2012 at 05:40:00PM +0000, Cong Ding wrote:
> > > > >> In version 1, I forgot to modify the same bug in the first loop.
> > > > >>
> > > > >> we have to call kobject_put() to clean up the kobject after function
> > > > >> kobject_init(), kobject_add(), or kobject_uevent() is called.
> > > > >>
> > > > >> Signed-off-by: Cong Ding <[email protected]>
> > > > >
> > > > > Signed-off-by: "Hans J. Koch" <[email protected]>
> > > >
> > > > Hi Greg, is this patch stil in the queue?
> > >
> > > Hans is queueing up UIO patches to send to me, I'm waiting for him to
> > > send them as I don't have any in my trees.
> >
> > I'll set that up tonight or tomorrow. Sorry, I was delayed by illness
> > and a lot of other work.
> Hi Hans, what's this going on?
>
> Thanks, - cong
ping
On Thu, Apr 25, 2013 at 12:19 PM, Cong Ding <[email protected]> wrote:
> On Thu, Feb 14, 2013 at 12:43:15PM +0100, Cong Ding wrote:
>> On Sun, Jan 20, 2013 at 10:01:41PM +0100, Hans J. Koch wrote:
>> > On Fri, Jan 18, 2013 at 02:00:50PM -0800, Greg Kroah-Hartman wrote:
>> > > On Fri, Jan 18, 2013 at 10:05:45PM +0100, Cong Ding wrote:
>> > > > On Tue, Dec 11, 2012 at 2:21 AM, Hans J. Koch <[email protected]> wrote:
>> > > > > On Thu, Nov 29, 2012 at 05:40:00PM +0000, Cong Ding wrote:
>> > > > >> In version 1, I forgot to modify the same bug in the first loop.
>> > > > >>
>> > > > >> we have to call kobject_put() to clean up the kobject after function
>> > > > >> kobject_init(), kobject_add(), or kobject_uevent() is called.
>> > > > >>
>> > > > >> Signed-off-by: Cong Ding <[email protected]>
>> > > > >
>> > > > > Signed-off-by: "Hans J. Koch" <[email protected]>
>> > > >
>> > > > Hi Greg, is this patch stil in the queue?
>> > >
>> > > Hans is queueing up UIO patches to send to me, I'm waiting for him to
>> > > send them as I don't have any in my trees.
>> >
>> > I'll set that up tonight or tomorrow. Sorry, I was delayed by illness
>> > and a lot of other work.
>> Hi Hans, what's this going on?
>>
>> Thanks, - cong
Hi Hans, it's another month. What's the progress of this patch?
- cong
On Mon, May 27, 2013 at 12:14 AM, Cong Ding <[email protected]> wrote:
> On Thu, Apr 25, 2013 at 12:19 PM, Cong Ding <[email protected]> wrote:
>> On Thu, Feb 14, 2013 at 12:43:15PM +0100, Cong Ding wrote:
>>> On Sun, Jan 20, 2013 at 10:01:41PM +0100, Hans J. Koch wrote:
>>> > On Fri, Jan 18, 2013 at 02:00:50PM -0800, Greg Kroah-Hartman wrote:
>>> > > On Fri, Jan 18, 2013 at 10:05:45PM +0100, Cong Ding wrote:
>>> > > > On Tue, Dec 11, 2012 at 2:21 AM, Hans J. Koch <[email protected]> wrote:
>>> > > > > On Thu, Nov 29, 2012 at 05:40:00PM +0000, Cong Ding wrote:
>>> > > > >> In version 1, I forgot to modify the same bug in the first loop.
>>> > > > >>
>>> > > > >> we have to call kobject_put() to clean up the kobject after function
>>> > > > >> kobject_init(), kobject_add(), or kobject_uevent() is called.
>>> > > > >>
>>> > > > >> Signed-off-by: Cong Ding <[email protected]>
>>> > > > >
>>> > > > > Signed-off-by: "Hans J. Koch" <[email protected]>
>>> > > >
>>> > > > Hi Greg, is this patch stil in the queue?
>>> > >
>>> > > Hans is queueing up UIO patches to send to me, I'm waiting for him to
>>> > > send them as I don't have any in my trees.
>>> >
>>> > I'll set that up tonight or tomorrow. Sorry, I was delayed by illness
>>> > and a lot of other work.
>>> Hi Hans, what's this going on?
>>>
>>> Thanks, - cong
> Hi Hans, it's another month. What's the progress of this patch?
still no action to this this patch? it was signed-off by Hans 7 months ago!
- cong