Prev: uml: i386: Avoid redefinition of NR_syscalls
Next: rcu: slim down rcutiny by removing rcu_scheduler_active and friends
From: Tomas Winkler on 25 Apr 2010 15:30
On Sun, Apr 25, 2010 at 7:37 PM, Greg KH <greg(a)kroah.com> wrote:
> On Thu, Apr 22, 2010 at 01:22:51AM +0300, Tomas Winkler wrote:
>> On Mon, Apr 19, 2010 at 5:59 PM, Greg KH <greg(a)kroah.com> wrote:
>> > On Mon, Apr 19, 2010 at 03:20:34PM +0300, Tomas Winkler wrote:
>> >> Lately we've been developing a device that rather more extensively
>> >> used request_firmware API in load and also using pm_notifiers to load
>> >> firmware.
>> >
>> > Do you have a pointer to your driver source anywhere that shows how you
>> > are trying to use the firmware api in this manner?
>>
>> I've attached a very simple  test driver I'm using.  Just wanted to
>> eliminate anything else.
>> Bellow is a little script that loads and releases the firmware. My
>> previous observation was wrong.
>> The free memory gradually decreases regardless of number or dangling
>> udevd forks, which are eventually collected if the sleep period is
>> long enough ~10s.
>
> That sounds normal for the free memory.  Kay, that's also to be expected
> for the udevd forks as well, right?

Sorry maybe I was not clear what I mean that the memory will be
eventually exhausted and system will crash
Is this normal? Actually I less suspect now udevd as the same happens
on android platform where there is no udev


Tomas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 25 Apr 2010 15:40
On Sun, Apr 25, 2010 at 10:22:54PM +0300, Tomas Winkler wrote:
> On Sun, Apr 25, 2010 at 7:37 PM, Greg KH <greg(a)kroah.com> wrote:
> > On Thu, Apr 22, 2010 at 01:22:51AM +0300, Tomas Winkler wrote:
> >> On Mon, Apr 19, 2010 at 5:59 PM, Greg KH <greg(a)kroah.com> wrote:
> >> > On Mon, Apr 19, 2010 at 03:20:34PM +0300, Tomas Winkler wrote:
> >> >> Lately we've been developing a device that rather more extensively
> >> >> used request_firmware API in load and also using pm_notifiers to load
> >> >> firmware.
> >> >
> >> > Do you have a pointer to your driver source anywhere that shows how you
> >> > are trying to use the firmware api in this manner?
> >>
> >> I've attached a very simple ??test driver I'm using. ??Just wanted to
> >> eliminate anything else.
> >> Bellow is a little script that loads and releases the firmware. My
> >> previous observation was wrong.
> >> The free memory gradually decreases regardless of number or dangling
> >> udevd forks, which are eventually collected if the sleep period is
> >> long enough ~10s.
> >
> > That sounds normal for the free memory. ??Kay, that's also to be expected
> > for the udevd forks as well, right?
>
> Sorry maybe I was not clear what I mean that the memory will be
> eventually exhausted and system will crash
> Is this normal?

Ah, no, that's not normal. Have you run kmemleak on your module (or
test module) to verify that you are properly freeing up the memory?

> Actually I less suspect now udevd as the same happens on android
> platform where there is no udev

Which is a sad thing for a whole other range of issues...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Tomas Winkler on 25 Apr 2010 16:20
On Sun, Apr 25, 2010 at 10:36 PM, Greg KH <greg(a)kroah.com> wrote:
> On Sun, Apr 25, 2010 at 10:22:54PM +0300, Tomas Winkler wrote:
>> On Sun, Apr 25, 2010 at 7:37 PM, Greg KH <greg(a)kroah.com> wrote:
>> > On Thu, Apr 22, 2010 at 01:22:51AM +0300, Tomas Winkler wrote:
>> >> On Mon, Apr 19, 2010 at 5:59 PM, Greg KH <greg(a)kroah.com> wrote:
>> >> > On Mon, Apr 19, 2010 at 03:20:34PM +0300, Tomas Winkler wrote:
>> >> >> Lately we've been developing a device that rather more extensively
>> >> >> used request_firmware API in load and also using pm_notifiers to load
>> >> >> firmware.
>> >> >
>> >> > Do you have a pointer to your driver source anywhere that shows how you
>> >> > are trying to use the firmware api in this manner?
>> >>
>> >> I've attached a very simple ??test driver I'm using. ??Just wanted to
>> >> eliminate anything else.
>> >> Bellow is a little script that loads and releases the firmware. My
>> >> previous observation was wrong.
>> >> The free memory gradually decreases regardless of number or dangling
>> >> udevd forks, which are eventually collected if the sleep period is
>> >> long enough ~10s.
>> >
>> > That sounds normal for the free memory. ??Kay, that's also to be expected
>> > for the udevd forks as well, right?
>>
>> Sorry maybe I was not clear what I mean that the memory will be
>> eventually exhausted and system will crash
>> Is this normal?
>
> Ah, no, that's not normal.  Have you run kmemleak on your module (or
> test module) to verify that you are properly freeing up the memory?

yes, one of my college has run the kmemleak but didn't bring much
evidence. I've also looked into slubinfo as suggested by Johannes
but don't see anything suspicions either but this is expected as
everything is allocated through kmalloc and alloc_pages.
I may rerun the kmemleak by myself but this shows up on too many
setups and kernels with also full driver and also my simple test
driver.

>
>> Actually I less suspect now udevd as the same happens on android
>> platform where there is no udev
>
> Which is a sad thing for a whole other range of issues...

Said thing is that I don't see where the memory goes.... Anyhow I will
try to run valgrin on udev just to be sure.

I'll be glad If someone can run my simple driver I posted and confirm
that sees the same problem

Thanks
Tomas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Kay Sievers on 26 Apr 2010 06:40
On Sun, Apr 25, 2010 at 22:09, Tomas Winkler <tomasw(a)gmail.com> wrote:
> Said thing is that I don't see where the memory goes.... Anyhow I will
> try to run valgrin on udev just to be sure.

Nah, that memory would be freed, if you kill all udev processes, which
it doesn't.

The many udev worker processes you see for a few seconds was caused by
udevd handling events with TIMEOUT= set special. We need to make sure,
that firmware events run immediately and don't wait for other
processes to finish. The logic who does that was always creating a new
worker. I changed this now, but this will not affect the underlying
problem you are seeing, it will just make the udev workers not grow in
a timeframe of less than 10 seconds. The change is here:
http://git.kernel.org/?p=linux/hotplug/udev.git;a=commit;h=665ee17def2caa6811ae032ae68ebf8239a18cf8
but as mentioned, this change is unrelated to the memory leak you are seeing.

> I'll be glad If someone can run my simple driver I posted and confirm
> that sees the same problem

I can confirm that memory gets lost. I suspect for some reason the
firmware does not get properly cleaned up. If you increase the size of
the firmware image, it will leak memory much faster.

Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Kay Sievers on 26 Apr 2010 11:30
On Mon, 2010-04-26 at 12:38 +0200, Kay Sievers wrote:
> On Sun, Apr 25, 2010 at 22:09, Tomas Winkler <tomasw(a)gmail.com> wrote:
> > Said thing is that I don't see where the memory goes.... Anyhow I will
> > try to run valgrin on udev just to be sure.
>
> Nah, that memory would be freed, if you kill all udev processes, which
> it doesn't.
>
> The many udev worker processes you see for a few seconds was caused by
> udevd handling events with TIMEOUT= set special. We need to make sure,
> that firmware events run immediately and don't wait for other
> processes to finish. The logic who does that was always creating a new
> worker. I changed this now, but this will not affect the underlying
> problem you are seeing, it will just make the udev workers not grow in
> a timeframe of less than 10 seconds. The change is here:
> http://git.kernel.org/?p=linux/hotplug/udev.git;a=commit;h=665ee17def2caa6811ae032ae68ebf8239a18cf8
> but as mentioned, this change is unrelated to the memory leak you are seeing.
>
> > I'll be glad If someone can run my simple driver I posted and confirm
> > that sees the same problem
>
> I can confirm that memory gets lost. I suspect for some reason the
> firmware does not get properly cleaned up. If you increase the size of
> the firmware image, it will leak memory much faster.

I guess, the assumption that vfree() will free pages which are allocated
by custom code, and not by vmalloc(), is not true.

The attached changes seem to fix the issue for me.

The custom page array mangling was introduced by David as an optimization
with commit 6e03a201bbe8137487f340d26aa662110e324b20 and this should be
checked, and if needed be fixed.

Cheers,
Kay


diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 985da11..fe4e872 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -162,7 +162,7 @@ static ssize_t firmware_loading_store(struct device *dev,
mutex_unlock(&fw_lock);
break;
}
- vfree(fw_priv->fw->data);
+ vunmap(fw_priv->fw->data);
fw_priv->fw->data = NULL;
for (i = 0; i < fw_priv->nr_pages; i++)
__free_page(fw_priv->pages[i]);
@@ -176,7 +176,7 @@ static ssize_t firmware_loading_store(struct device *dev,
break;
case 0:
if (test_bit(FW_STATUS_LOADING, &fw_priv->status)) {
- vfree(fw_priv->fw->data);
+ vunmap(fw_priv->fw->data);
fw_priv->fw->data = vmap(fw_priv->pages,
fw_priv->nr_pages,
0, PAGE_KERNEL_RO);
@@ -184,9 +184,6 @@ static ssize_t firmware_loading_store(struct device *dev,
dev_err(dev, "%s: vmap() failed\n", __func__);
goto err;
}
- /* Pages will be freed by vfree() */
- fw_priv->page_array_size = 0;
- fw_priv->nr_pages = 0;
complete(&fw_priv->completion);
clear_bit(FW_STATUS_LOADING, &fw_priv->status);
break;
@@ -578,7 +575,7 @@ release_firmware(const struct firmware *fw)
if (fw->data == builtin->data)
goto free_fw;
}
- vfree(fw->data);
+ vunmap(fw->data);
free_fw:
kfree(fw);
}


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  |  Next  |  Last
Pages: 1 2 3 4
Prev: uml: i386: Avoid redefinition of NR_syscalls
Next: rcu: slim down rcutiny by removing rcu_scheduler_active and friends