> On Sep 5, 2025, at 18:28, Kiryl Shutsemau 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 >>> --- >> >> ... >>> +#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 > >> Tested-by: wang lian > >> >> Best regards, >> wang lian > > -- > Kiryl Shutsemau / Kirill A. Shutemov