Re: [LTP] [PATCH v6 1/2] mem/lib: add vm tunable file read/write/check func

Caspar Zhang
2011-12-28T08:10:29Z

On 12/28/2011 03:48 PM, Wanlong Gao wrote:
> On 12/28/2011 02:47 PM, Caspar Zhang wrote:
>
>> On 11/15/2011 09:14 PM, Zhouping Liu wrote:
>>> hi, Cyril & Garrett
>>> v5 -> v6: updated the sscanf() function in read_meminfo() in order to avoid buffer overflow.
>>>
>>> please feel free to comment the patch.
>>> BTW: you would miss the patch.
>>>
>>> There are several vm tunable files under /proc/sys/vm, I will compose
>>> some cases for testing the basic functions of them. This patch is adding
>>> the read/write/check functions to mem lib so that I can include them in
>>> my testcases.
>>>
>>> set_sys_tune(): set a long long integer value to a vm tunable file.
>>> get_sys_tune(): get a long long integer value from a vm tunable file.
>>> check_sys_tune(): check to confirm the value in tunable file meets our
>>> expectation.
>>> read_meminfo(): read the special value from /proc/meminfo.
>>>
>>> Since most of the values in vm tunable files contain only single
>>> integer, the functions I added only read/write long long values for now.
>>> For those files not storing single interger (e.g. lowmem_reserve_ratio),
>>> these functions will not be used in the case.
>>>
>>> Signed-off-by: Zhouping Liu <zliu@redhat.com>
>>> ---
>>> runtest/mm | 7 +++
>>> testcases/kernel/mem/include/mem.h | 7 +++
>>> testcases/kernel/mem/lib/mem.c | 84 +++++++++++++++++++++++++++++++++++-
>>> 3 files changed, 97 insertions(+), 1 deletions(-)
>>
>> After talking to Zhouping Liu, I committed this patch with some
>> modifications.
>
>
> I saw that the patch you just committed defined an unused "PATH_SYSVM",
> but as your comment of this patch, the set/get functions is just for sysfiles
> under PATH_SYSVM, if so, you should do things like below first?
>
> snprintf(buf, BUFSIZ, "%s%s", PATH_KSM, path);

Hi, the original design was to pass full path to set/get_sys_tune()
functions, PATH_SYSVM will be used in other files. I agree with your
idea, it looks like sys_file is better to be filename only. I'll merge
your request.

Thanks,
Caspar