[Accel-config] Re: [RFC] Provide library API to read and write raw sysfs attributes

Ramesh Thomas <ramesh.thomas at intel.com> Mon, 07 Feb 2022 13:34:06 -0800
Newsgroups dev.linux.lists.accel-config
Message-ID <[email protected]>
On 2/4/2022 3:26 PM, Dave Jiang wrote:
> 
> On 2/4/2022 2:03 PM, Ramesh Thomas wrote:
>> On 2/4/2022 12:42 PM, Dave Jiang wrote:
>>>
>>> On 2/4/2022 12:15 PM, Ramesh Thomas wrote:
>>>> I am thinking of providing some generic functions to read and write 
>>>> sysfs attributes without much processing. This will be useful where 
>>>> raw reads and writes are required e.g. listing values of attributes 
>>>> or initializing attributes in a loop. This will open up 
>>>> possibilities of advanced use cases where user can do custom batch 
>>>> configurations in addition to the load and save configuration option.
>>>>
>>>> There will be a get and set function for devices, groups, wqs and 
>>>> engines as follows
>>>>
>>>> /* structure to read and write attributes from sysfs */
>>>> struct accfg_sysfs_attr {
>>>>         char attr[MAX_PARAM_LEN];
>>>> };
>>>>
>>>> int accfg_get_device_attr(struct accfg_device *device, const char 
>>>> *attr_name,
>>>>                 struct *accfg_sysfs_attr);
>>>> int accfg_set_device_attr(struct accfg_device *device, const char 
>>>> *attr_name,
>>>>                 struct *accfg_sysfs_attr);
>>>>
>>>> int accfg_get_group_attr(struct accfg_group *group, const char 
>>>> *attr_name,
>>>>                 struct *accfg_sysfs_attr);
>>>> int accfg_set_group_attr(struct accfg_group *group, const char 
>>>> *attr_name,
>>>>                 struct *accfg_sysfs_attr);
>>>>
>>> Does it provide additional processing on top of sysfs_read_attr() 
>>> helper function?
>>
>> It creates the path from from device, wq, engine and group structure. 
>> It will save last error (cmd_status) in accfg ctxt and return errors 
>> returned from driver. Other than that it does not do any attribute 
>> specific processing.
> 
> Ok by me from high level view if it makes the code cleaner.

Unfortunately this will not work. The intention was to provide a generic 
API to read/write sysfs attributes as strings. However since we cache 
attributes in their respective types (int, long, string etc.), attribute 
specific processing cannot be avoided.

Maybe in future we could simplify things by not caching values in 
private structures. I don't see a reason to cache them when that is done 
in the sysfs and persisted in config files. We only need to validate 
when user enters a value and load all of them in json as strings.

> 
> 
>>
>>>
>>>
>>>> similarly for wqs and engines.
>>>>
>>>> Let me know if you see any issue or other suggestions.
>>>>
>>>> Thanks,
>>>> Ramesh
>>