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

On 2/7/2022 1:43 PM, Dave Jiang wrote:
> 
> On 2/7/2022 2:34 PM, Ramesh Thomas wrote:
>> 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.
> 
> I think the issue is with apps using the libaccel-config API. If an 
> attribute is persistent, then you don't want to keep pulling it from 
> sysfs when you can read it locally. Especially if something is needed 
> for the fast I/O path.

Since this is only a configuration tool, there is no need for fast I/O. 
We can store all the attributes in json as strings.

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