lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2545ff50-8e59-ff9f-1ba0-cb2661d5119a@samsung.com>
Date:   Tue, 26 Nov 2019 18:37:29 +0900
From:   Chanwoo Choi <cw00.choi@...sung.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        rafael.j.wysocki@...el.com, myungjoo.ham@...sung.com,
        kyungmin.park@...sung.com, chanwoo@...nel.org,
        stable@...r.kernel.org
Subject: Re: [PATCH v3] PM / devfreq: Add new name attribute for sysfs

On 11/26/19 6:15 PM, Greg KH wrote:
> On Tue, Nov 26, 2019 at 05:35:28PM +0900, Chanwoo Choi wrote:
>> On 11/26/19 4:53 PM, Greg KH wrote:
>>> On Tue, Nov 26, 2019 at 12:08:18PM +0900, Chanwoo Choi wrote:
>>>> Hi Greg,
>>>>
>>>> On 11/25/19 5:50 PM, Greg KH wrote:
>>>>> On Mon, Nov 25, 2019 at 10:03:57AM +0900, Chanwoo Choi wrote:
>>>>>> The commit 4585fbcb5331 ("PM / devfreq: Modify the device name as devfreq(X) for
>>>>>> sysfs") changed the node name to devfreq(x). After this commit, it is not
>>>>>> possible to get the device name through /sys/class/devfreq/devfreq(X)/*.
>>>>>>
>>>>>> Add new name attribute in order to get device name.
>>>>>>
>>>>>> Cc: stable@...r.kernel.org
>>>>>> Fixes: 4585fbcb5331 ("PM / devfreq: Modify the device name as devfreq(X) for sysfs")
>>>>>> Signed-off-by: Chanwoo Choi <cw00.choi@...sung.com>
>>>>>> ---
>>>>>>  Changes from v2:
>>>>>> - Change the order of name_show() according to the sequence in devfreq_attrs[]
>>>>>>
>>>>>> Changes from v1:
>>>>>> - Update sysfs-class-devfreq documentation
>>>>>> - Show device name directly from 'devfreq->dev.parent'
>>>>>>
>>>>>
>>>>> Shouldn't you just revert the original patch here?  Why did the sysfs
>>>>> file change?
>>>>
>>>> The initial devfreq code used the parent device name for device name
>>>> corresponding to devfreq object instead of 'devfreq%d' style.
>>>> Before applied The commit 4585fbcb5331 ("PM / devfreq: Modify
>>>> the device name as devfreq(X) for sysfs"), the devfreq sysfs
>>>> showed the parent device name as following:
>>>>
>>>> For example on Odroid-XU3 board before applied the commit 4585fbcb5331,
>>>> 	/sys/class/devfreq/soc:bus_wcore
>>>> 	/sys/class/devfreq/soc:bus_noc
>>>> 	...(skip)
>>>>
>>>>
>>>> But, I think that devfreq subsystem had to show the consistent
>>>> sysfs entry name for devfreq device like input, thermal, hwmon subsystem.
>>>>
>>>> For example on Odroid-XU3 board,
>>>> - The input subsystem show the 'input%d' style for input device.
>>>> $root@...alhost:/# ls /sys/class/input/                                         
>>>> event0  event1  input0  input1  mice  mouse0
>>>>
>>>> - The thermal subsystem show the 'cooling_device%d' style for thermal cooling device.
>>>> $ root@...alhost:/# ls /sys/class/thermal/                                       
>>>> cooling_device0  cooling_device2  thermal_zone1  thermal_zone3
>>>> cooling_device1  thermal_zone0    thermal_zone2  thermal_zone4
>>>>
>>>> - The hwmon subsystem show the 'hwmon%d' style for h/w monitor device.
>>>> $root@...alhost:/# ls /sys/class/hwmon/                                         
>>>> hwmon0
>>>>
>>>>
>>>> So, I tried to make the consistent sysfs entry name for devfreq device
>>>> by contributing commit 4585fbcb5331 ("PM / devfreq: Modify the device name as
>>>> devfreq(X) for sysfs"). But, The commit 4585fbcb5331 have missed that sysfs
>>>> interface had to provide the real device name. Some subsystem like thermal
>>>> and hwmon provide the device type or device name through sysfs interface.
>>>> It is possible to make the user to find their own specific device by iteration
>>>> on user-space.
>>>>
>>>> root@...alhost:/# cat /sys/class/thermal/cooling_device0/type 
>>>> pwm-fan
>>>> root@...alhost:/# cat /sys/class/thermal/cooling_device1/type                  
>>>> thermal-cpufreq-0
>>>> root@...alhost:/# cat /sys/class/thermal/cooling_device2/type                  
>>>> thermal-cpufreq-1
>>>>
>>>> root@...alhost:/# cat /sys/class/hwmon/hwmon0/name                             
>>>> pwmfan
>>>>
>>>>
>>>> So, I add the new 'name' attribute of sysfs for devfreq device.
>>>>
>>>>>
>>>>>> Documentation/ABI/testing/sysfs-class-devfreq | 7 +++++++
>>>>>>  drivers/devfreq/devfreq.c                     | 9 +++++++++
>>>>>>  2 files changed, 16 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq
>>>>>> index 01196e19afca..75897e2fde43 100644
>>>>>> --- a/Documentation/ABI/testing/sysfs-class-devfreq
>>>>>> +++ b/Documentation/ABI/testing/sysfs-class-devfreq
>>>>>> @@ -7,6 +7,13 @@ Description:
>>>>>>  		The name of devfreq object denoted as ... is same as the
>>>>>>  		name of device using devfreq.
>>>>>>  
>>>>>> +What:		/sys/class/devfreq/.../name
>>>>>> +Date:		November 2019
>>>>>> +Contact:	Chanwoo Choi <cw00.choi@...sung.com>
>>>>>> +Description:
>>>>>> +		The /sys/class/devfreq/.../name shows the name of device
>>>>>> +		of the corresponding devfreq object.
>>>>>> +
>>>>>>  What:		/sys/class/devfreq/.../governor
>>>>>>  Date:		September 2011
>>>>>>  Contact:	MyungJoo Ham <myungjoo.ham@...sung.com>
>>>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>>>>> index 65a4b6cf3fa5..6f4d93d2a651 100644
>>>>>> --- a/drivers/devfreq/devfreq.c
>>>>>> +++ b/drivers/devfreq/devfreq.c
>>>>>> @@ -1169,6 +1169,14 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
>>>>>>  }
>>>>>>  EXPORT_SYMBOL(devfreq_remove_governor);
>>>>>>  
>>>>>> +static ssize_t name_show(struct device *dev,
>>>>>> +			struct device_attribute *attr, char *buf)
>>>>>> +{
>>>>>> +	struct devfreq *devfreq = to_devfreq(dev);
>>>>>> +	return sprintf(buf, "%s\n", dev_name(devfreq->dev.parent));
>>>>>
>>>>> Why is the parent's name being set here, and not the device name?
>>>>
>>>> The device name style in struct devfreq is 'devfreq%d' instead of
>>>> parent device name in order to keep the consistent naming style for devfreq device
>>>> as I mentioned above. 'devfreq%d' name is consistent name style for devfreq device.
>>>> But, it don't show the real h/w device name. So, show the parent device name
>>>> which is specified on device-tree file.
>>>
>>> I'm sorry, but I still do not understand.  Can you show me the directory
>>> tree before and after here?
>>>
>>
>> I'm sorry for not enough description. I add the following example on Odroid-XU3 board.
>>
>>
>> 1. Before applied commit 4585fbcb5331 ("PM / devfreq: Modify the device name as devfreq(X),
>>
>> root@...alhost:~# ls /sys/class/devfreq                                        
>> soc:bus_disp1       soc:bus_fsys_apb  soc:bus_gscl_scaler  soc:bus_mscl
>> soc:bus_disp1_fimd  soc:bus_g2d       soc:bus_jpeg         soc:bus_noc
>> soc:bus_fsys        soc:bus_g2d_acp   soc:bus_jpeg_apb     soc:bus_peri
>> soc:bus_fsys2       soc:bus_gen       soc:bus_mfc          soc:bus_wcore
>>
>> root@...alhost:~# ls -al /sys/class/devfreq
>> total 0
>> drwxr-xr-x  2 root root 0 Jan  1 09:00 .
>> drwxr-xr-x 52 root root 0 Jan  1 09:00 ..
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_disp1 -> ../../devices/platform/soc/soc:bus_disp1/devfreq/soc:bus_disp1
> 
> Ah, that's odd, ok.
> 
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_disp1_fimd -> ../../devices/platform/soc/soc:bus_disp1_fimd/devfreq/soc:bus_did
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_fsys -> ../../devices/platform/soc/soc:bus_fsys/devfreq/soc:bus_fsys
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_fsys2 -> ../../devices/platform/soc/soc:bus_fsys2/devfreq/soc:bus_fsys2
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_fsys_apb -> ../../devices/platform/soc/soc:bus_fsys_apb/devfreq/soc:bus_fsys_ab
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_g2d -> ../../devices/platform/soc/soc:bus_g2d/devfreq/soc:bus_g2d
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_g2d_acp -> ../../devices/platform/soc/soc:bus_g2d_acp/devfreq/soc:bus_g2d_acp
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_gen -> ../../devices/platform/soc/soc:bus_gen/devfreq/soc:bus_gen
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_gscl_scaler -> ../../devices/platform/soc/soc:bus_gscl_scaler/devfreq/soc:bus_r
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_jpeg -> ../../devices/platform/soc/soc:bus_jpeg/devfreq/soc:bus_jpeg
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_jpeg_apb -> ../../devices/platform/soc/soc:bus_jpeg_apb/devfreq/soc:bus_jpeg_ab
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_mfc -> ../../devices/platform/soc/soc:bus_mfc/devfreq/soc:bus_mfc
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_mscl -> ../../devices/platform/soc/soc:bus_mscl/devfreq/soc:bus_mscl
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_noc -> ../../devices/platform/soc/soc:bus_noc/devfreq/soc:bus_noc
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_peri -> ../../devices/platform/soc/soc:bus_peri/devfreq/soc:bus_peri
>> lrwxrwxrwx  1 root root 0 Jan  1 09:00 soc:bus_wcore -> ../../devices/platform/soc/soc:bus_wcore/devfreq/soc:bus_wcore
>>
>>
>>
>> 2. After applied commit 4585fbcb5331 ("PM / devfreq: Modify the device name as devfreq(X),
>>
>> root@...alhost:~# ls  /sys/class/devfreq                                       
>> devfreq0   devfreq11  devfreq14  devfreq3  devfreq6  devfreq9
>> devfreq1   devfreq12  devfreq15  devfreq4  devfreq7
>> devfreq10  devfreq13  devfreq2   devfreq5  devfreq8
> 
> That's better.
> 
>>
>> root@...alhost:~# ls -al /sys/class/devfreq                                    
>> total 0
>> drwxr-xr-x  2 root root 0 Jan  1 09:02 .
>> drwxr-xr-x 52 root root 0 Jan  1 09:02 ..
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq0 -> ../../devices/platform/soc/soc:bus_wcore/devfreq/devfreq0
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq1 -> ../../devices/platform/soc/soc:bus_noc/devfreq/devfreq1
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq10 -> ../../devices/platform/soc/soc:bus_jpeg/devfreq/devfreq10
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq11 -> ../../devices/platform/soc/soc:bus_jpeg_apb/devfreq/devfreq11
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq12 -> ../../devices/platform/soc/soc:bus_disp1_fimd/devfreq/devfreq12
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq13 -> ../../devices/platform/soc/soc:bus_disp1/devfreq/devfreq13
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq14 -> ../../devices/platform/soc/soc:bus_gscl_scaler/devfreq/devfreq14
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq15 -> ../../devices/platform/soc/soc:bus_mscl/devfreq/devfreq15
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq2 -> ../../devices/platform/soc/soc:bus_fsys_apb/devfreq/devfreq2
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq3 -> ../../devices/platform/soc/soc:bus_fsys/devfreq/devfreq3
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq4 -> ../../devices/platform/soc/soc:bus_fsys2/devfreq/devfreq4
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq5 -> ../../devices/platform/soc/soc:bus_mfc/devfreq/devfreq5
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq6 -> ../../devices/platform/soc/soc:bus_gen/devfreq/devfreq6
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq7 -> ../../devices/platform/soc/soc:bus_peri/devfreq/devfreq7
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq8 -> ../../devices/platform/soc/soc:bus_g2d/devfreq/devfreq8
>> lrwxrwxrwx  1 root root 0 Jan  1 09:02 devfreq9 -> ../../devices/platform/soc/soc:bus_g2d_acp/devfreq/devfreq9
> 
> Ok, this looks a bit better, but why is there the extra "devfreq"
> directory in there?  That was in the original as well, but that feels
> odd.

What extra directory are you talking about? I didn't create
the any extra directory for devfreq.

If you mention the following 'devfreq' directory, 
it is created by basic device driver code in linux kernel.
- /sys/devices/platform/soc/soc\:bus_wcore/devfreq/

or

Each devfreq0~15 directories indicates the each devfreq device.


For example, show the info of each path
root@...alhost:~# ls -al /sys/devices/platform/soc/soc\:bus_wcore/             
total 0
drwxr-xr-x   4 root root    0 Jan  1 09:56 .
drwxr-xr-x 109 root root    0 Jan  1 09:56 ..
drwxr-xr-x   3 root root    0 Jan  1 09:56 devfreq
lrwxrwxrwx   1 root root    0 Jan  1 09:57 driver -> ../../../../bus/platform/drivers/exynos-bus
-rw-r--r--   1 root root 4096 Jan  1 09:57 driver_override
-r--r--r--   1 root root 4096 Jan  1 09:57 modalias
lrwxrwxrwx   1 root root    0 Jan  1 09:57 of_node -> ../../../../firmware/devicetree/base/soc/bus_wcore
drwxr-xr-x   2 root root    0 Jan  1 09:57 power
lrwxrwxrwx   1 root root    0 Jan  1 09:56 subsystem -> ../../../../bus/platform
-rw-r--r--   1 root root 4096 Jan  1 09:56 uevent

root@...alhost:~# ls -al /sys/devices/platform/soc/soc\:bus_wcore/devfreq/     
total 0
drwxr-xr-x 3 root root 0 Jan  1 09:56 .
drwxr-xr-x 4 root root 0 Jan  1 09:56 ..
drwxr-xr-x 3 root root 0 Jan  1 09:56 devfreq0

root@...alhost:~# ls -al /sys/devices/platform/soc/soc\:bus_wcore/devfreq/devfreq0
drwxr-xr-x 3 root root    0 Jan  1 09:56 .
drwxr-xr-x 3 root root    0 Jan  1 09:56 ..
-r--r--r-- 1 root root 4096 Jan  1 10:03 available_frequencies
-r--r--r-- 1 root root 4096 Jan  1 10:03 available_governors
-r--r--r-- 1 root root 4096 Jan  1 10:03 cur_freq
lrwxrwxrwx 1 root root    0 Jan  1 10:03 device -> ../../../soc:bus_wcore
-rw-r--r-- 1 root root 4096 Jan  1 10:03 governor
-rw-r--r-- 1 root root 4096 Jan  1 10:03 max_freq
-rw-r--r-- 1 root root 4096 Jan  1 10:03 min_freq
-r--r--r-- 1 root root 4096 Jan  1 10:03 name
-rw-r--r-- 1 root root 4096 Jan  1 10:03 polling_interval
drwxr-xr-x 2 root root    0 Jan  1 10:03 power
lrwxrwxrwx 1 root root    0 Jan  1 09:56 subsystem -> ../../../../../../class/devfreq
-r--r--r-- 1 root root 4096 Jan  1 10:03 target_freq
-r--r--r-- 1 root root 4096 Jan  1 10:03 trans_stat
-rw-r--r-- 1 root root 4096 Jan  1 09:56 uevent




-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ