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

[Kiryl Shutsemau <kirill@shutemov.name>]

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.

> Other than that, the patch looks great to me.
> LGTM!
> Reviewed-by: wang lian <lianux.mm@gmail.com>
> Tested-by: wang lian <lianux.mm@gmail.com>
> 
> Best regards,
> wang lian

-- 
  Kiryl Shutsemau / Kirill A. Shutemov