Re: [LTP] [PATCH 1/1] lib: Skip tst_{disable, enable}_oom_protection() for non-root

["xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com>]

Hi Li
> Hi Xu, Petr,
>
> On Wed, Dec 22, 2021 at 10:26 AM xuyang2018.jy@fujitsu.com
> <mailto:xuyang2018.jy@fujitsu.com> <xuyang2018.jy@fujitsu.com
> <mailto:xuyang2018.jy@fujitsu.com>> wrote:
>
>     Hi Petr
>      > If needed to set value also for non-root, use set_oom_score_adj().
>     If so, why not rename set_oom_score_adj to tst_set_oom_score_adj and
>     add
>     declartion to tst_memutils.h?
>
>
> Yes, it makes sense to expose this function to users to cover
> more oom test scenarios. For instance, set a high (>0) or low (<0)
> score in child_alloc() to verify if OOM-Killer still works well.
> But so far, we don't have such tests.
>
>
>     ps: also have a word typo in set_oom_score_adj, adjustement =>
>     adjustment.
>
>     Best Regards
>     Yang Xu
>      >
>      > Fixes: 8a0827766d ("lib: add functions to adjust oom score")
>      >
>      > Signed-off-by: Petr Vorel<pvorel@suse.cz <mailto:pvorel@suse.cz>>
>      > ---
>      > include/tst_memutils.h | 11 ++++++++++-
>      > lib/tst_memutils.c | 6 ++++++
>      > 2 files changed, 16 insertions(+), 1 deletion(-)
>      >
>      > diff --git a/include/tst_memutils.h b/include/tst_memutils.h
>      > index 68a6e37714..e6f946ac0c 100644
>      > --- a/include/tst_memutils.h
>      > +++ b/include/tst_memutils.h
>      > @@ -30,11 +30,15 @@ long long tst_available_mem(void);
>      > * echo -1000>/proc/$PID/oom_score_adj
>      > * If the pid is 0 which means it will set on current(self) process.
>      > *
>      > + * WARNING:
>      > + * Do nothing for non-root, because setting value< 0 requires root.
>      > + If you want to set value also for non-root, use
>     set_oom_score_adj().
>      > + *
>      > * Note:
>      > * This exported tst_enable_oom_protection function can be used at
>     anywhere
>      > * you want to protect, but please remember that if you do enable
>     protection
>      > * on a process($PID) that all the children will inherit its score
>     and be
>      > - * ignored by OOM Killer as well. So that's why
>     tst_disable_oom_protection
>      > + * ignored by OOM Killer as well. So that's why
>     tst_disable_oom_protection()
>      > * to be used in combination.
>      > */
>      > void tst_enable_oom_protection(pid_t pid);
>      > @@ -42,6 +46,11 @@ void tst_enable_oom_protection(pid_t pid);
>      > /*
>      > * Disable the OOM protection for the process($PID).
>      > * echo 0>/proc/$PID/oom_score_adj
>      > + *
>      > + * WARNING:
>      > + * Do nothing for non-root, because it's expected to be cleanup
>     after
>      > + * tst_enable_oom_protection(). Use set_oom_score_adj(), if you
>     want to set
>      > + * value also for non-root.
>      > */
>      > void tst_disable_oom_protection(pid_t pid);
>      >
>      > diff --git a/lib/tst_memutils.c b/lib/tst_memutils.c
>      > index 4346508d9a..f0695e026a 100644
>      > --- a/lib/tst_memutils.c
>      > +++ b/lib/tst_memutils.c
>      > @@ -126,10 +126,16 @@ static void set_oom_score_adj(pid_t pid,
>     int value)
>      >
>      > void tst_enable_oom_protection(pid_t pid)
>      > {
>      > + if (geteuid() != 0)
>
>
> This is not working as expected in Github CI. I'm still looking at the
> problem.
> https://github.com/wangli5665/ltp/runs/4602025797?check_suite_focus=true
I tested local but it works well. I guess ci fails because of linux user 
namespace. Maybe we should require CAP_SYS_RESOURCE cap instead of using 
geteuid.

Best Regards
Yang Xu
>
> And the worth mentioning, maybe better to do this check
> in set_oom_score_adj() if we do not decide to expose
> that function to user.
>
> --
> Regards,
> Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp