Re: [PATCH] tools/mm: Add madvise tool

[wang lian <lianux.mm@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]



> On Sep 5, 2025, at 18:28, Kiryl Shutsemau <kirill@shutemov.name> wrote:
> 
> On Fri, Sep 05, 2025 at 11:26:13AM +0800, wang lian wrote:
>> 
>>> Add a simple tool that allows to issue an advice on a process or a file.
>> 
>>> It can be useful to experiment with effects of an advice on a workload
>>> without modifying the workload itself.
>> 
>>> Only supports advices available for process_madvise().
>> 
>>> Signed-off-by: Kiryl Shutsemau <kas@kernel.org>
>>> ---
>> 
>> ...
>>> +#define PMD_SIZE_FILE_PATH "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size"
>> 
>> Hi Kiryl,
>> 
>> Thanks for this patch, this is a very nice and useful tool.
>> 
>> I just have some super minor nitpick on coding style:
>> (a) Maybe move the #define PMD_SIZE_FILE_PATH to the top of the file, 
>> right after the #include statements?
> 
> Maybe. Will do if v2 is required.
> 
>> (b) use consistent comment style like this /* */ 
> 
> See answer to David.
> 
>> (c) when i test, when process_madvise test fail it give some message,
>> i think it should add into in the prompt message ,like this 
>>> printf("madvise TARGET ADVICE START END\n\n");
>> printf("should run as root\n\n");
>> printf("\t\tStart and end addressed for the advice. Must be page-aligned and valid target pid address.\n\n");
> 
> It doesn't need root. If your target is PID, it requires the target
> process to be ptraceable for you. Whether it is or not depending on your
> setup.

Thanks for your clarify.
> 
>> Other than that, the patch looks great to me.
>> LGTM!
>> Reviewed-by: wang lian <lianux.mm@gmail.com <mailto:lianux.mm@gmail.com>>
>> Tested-by: wang lian <lianux.mm@gmail.com <mailto:lianux.mm@gmail.com>>
>> 
>> Best regards,
>> wang lian
> 
> -- 
>  Kiryl Shutsemau / Kirill A. Shutemov


[-- Attachment #2: Type: text/html, Size: 11542 bytes --]