On Fri, Sep 16, 2016 at 08:49:44PM +0200, Jacek Anaszewski wrote: > On 09/16/2016 04:39 PM, Greg KH wrote: > > On Fri, Sep 16, 2016 at 04:32:39PM +0200, Jacek Anaszewski wrote: > > > On 09/16/2016 04:06 PM, Greg KH wrote: > > > > On Fri, Sep 16, 2016 at 03:41:09PM +0200, Jacek Anaszewski wrote: > > > > > On 09/16/2016 02:08 PM, Daniel Gorsulowski wrote: > > > > > > Hi Jacek, > > > > > > > > > > > > Am 16.09.2016 um 13:25 schrieb Jacek Anaszewski: > > > > > > > On 09/16/2016 10:15 AM, Daniel Gorsulowski wrote: > > > > > > > > Hi Jacek, > > > > > > > > > > > > > > > > Am 16.09.2016 um 09:31 schrieb Jacek Anaszewski: > > > > > > > > > Hi Daniel, > > > > > > > > > > > > > > > > > > On 09/12/2016 10:50 AM, Daniel Gorsulowski wrote: > > > > > > > > > > Hello! > > > > > > > > > > > > > > > > > > > > Please consider if I made something wrong, sending this issue. This is > > > > > > > > > > my first contact to the LKML. > > > > > > > > > > By mistake, I accessed an LED via /sys/class/leds subsystem very > > > > > > > > > > fast in > > > > > > > > > > an user application. I figured out, that the free user memory > > > > > > > > > > decreased > > > > > > > > > > constantly. So I tried to analyze the Problem and wrote a litte > > > > > > > > > > script: > > > > > > > > > > > > > > > > > > > > #!/bin/sh > > > > > > > > > > while [ 1 ]; do > > > > > > > > > > echo 1 > /sys/class/leds/2a_service_yellow/brightness > > > > > > > > > > echo 0 > /sys/class/leds/2a_service_yellow/brightness > > > > > > > > > > done > > > > > > > > > > > > > > > > > > > > And voila, I was able to reproduce the problem. > > > > > > > > > > So I add a bit more debugging: > > > > > > > > > > > > > > > > > > > > #!/bin/sh > > > > > > > > > > cnt=0 > > > > > > > > > > while [ 1 ]; do > > > > > > > > > > if [ `expr $cnt % 1000` -eq 0 ]; then > > > > > > > > > > free | grep Mem: | cut -d' ' -f25 > > > > > > > > > > fi > > > > > > > > > > echo 1 > /sys/class/leds/2a_service_yellow/brightness > > > > > > > > > > echo 0 > /sys/class/leds/2a_service_yellow/brightness > > > > > > > > > > let "cnt++" > > > > > > > > > > done > > > > > > > > > > > > > > > > > > > > And huh? No memory is eaten anymore. So it looks like, the problem > > > > > > > > > > only > > > > > > > > > > occours on heavy (fast) usage of /sys/class/leds subsystem. > > > > > > > > > > > > > > > > > > > > I rewrote the script and toggled a GPIO pin, but there was no problem > > > > > > > > > > recognizable. > > > > > > > > > > > > > > > > > > I've been unable to reproduce the problem with leds-aat1290 driver > > > > > > > > > and Samsung M0 board. It must be driver specific issue. > > > > > > > > > What driver did you use? > > > > > > > > > > > > > > > > > I defined LEDS_GPIO and so I'm using leds-gpio driver. > > > > > > > > danielg@debby:~/opt/prj/ti-linux-kernel$ cat .config | grep LEDS | grep > > > > > > > > -v "^# " > > > > > > > > CONFIG_INPUT_LEDS=y > > > > > > > > CONFIG_NEW_LEDS=y > > > > > > > > CONFIG_LEDS_CLASS=y > > > > > > > > CONFIG_LEDS_GPIO=y > > > > > > > > CONFIG_LEDS_TRIGGERS=y > > > > > > > > CONFIG_LEDS_TRIGGER_TIMER=y > > > > > > > > CONFIG_LEDS_TRIGGER_ONESHOT=y > > > > > > > > CONFIG_LEDS_TRIGGER_HEARTBEAT=y > > > > > > > > CONFIG_LEDS_TRIGGER_GPIO=y > > > > > > > > CONFIG_LEDS_TRIGGER_DEFAULT_ON=y > > > > > > > > CONFIG_LEDS_TRIGGER_TRANSIENT=y > > > > > > > > > > > > > > > > > > > > > > Unfortunately I am still unable to reproduce the problem with leds-gpio. > > > > > > > I'm not observing any heavy usage with your test case: > > > > > > > > > > > > > > ~#free > > > > > > > total used free shared buffers > > > > > > > cached > > > > > > > Mem: 1028092 61364 966728 0 8416 22396 > > > > > > > -/+ buffers/cache: 30552 997540 > > > > > > > Swap: 0 0 0 > > > > > > > > > > > > > > > > > > > > > Actually you didn't give any numbers. What kernel version are you using? > > > > > > > > > > > > > As I wrote, the problems occurred in vanilla 4.6 kernel, but also in 4.4 > > > > > > kernel (with PREEMPT-RT Patchset). > > > > > > > > > > Heh, funny coincidence. I was testing this on recent linux-leds.git, > > > > > for-next branch and was not able to detect the issue. It started to > > > > > appear after resetting HEAD to 4.8-rc2 base. Finally it turned out > > > > > that what fixes the issue is the most recent commit [1]. > > > > > > > > > > Further investigation revealed that this is kobject_uevent_env(), > > > > > called from led_trigger_set(), which causes memory leaks when called > > > > > with high frequency. > > > > > > > > Really? Where in kobject_uevent_env() is the memory leak? > > > > > > I'll chase it down when and will let you know. This may be > > > non-trivial issue as it suffices to add "sleep 0.1" between > > > brightness setting operations to prevent it. > > > > Why are you abusing uevents for flashing an LED? Please don't do that, > > it's not what that interface is for at all. > > It is called in a result of setting brightness value to LED_OFF, > which also removes registered trigger if any. So every time the LED goes off a uevent happens? That's not a good design. > The rationale for calling kobject_uevent_env() is given in the > relevant commit message: > > commit 52c47742f79d9240f90af9a6722fe8bb3fa8c0f9 > Author: Colin Cross <ccross@xxxxxxxxxxx> > Date: Mon Aug 27 09:31:49 2012 +0800 > > leds: triggers: send uevent when changing triggers > > Some triggers create sysfs files when they are enabled. Send a uevent > "change" notification whenever the trigger is changed to allow userspace > processes such as udev to modify permissions on the new files. > > A change notification will also be sent during registration of led class > devices or led triggers if the default trigger of an led class device > is found. If a sysfs file is removed, then I could see a change event being ok. But that's not what this patch does, it ALWAYS sends a uevent, even if nothing changed! Please fix that, otherwise you are going to really annoy userspace tools with this. But even then, I don't see how the uevent code has a memory leak with this, do you? And why aren't you checking the return value of kobject_uevent_env()? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
