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

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.


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