Re: [LTP] [PATCH] mem: make use of save_restore to simplify code

[Li Wang <liwang@redhat.com>]

On Thu, Jun 22, 2023 at 6:06 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> ...
> > > Also third parameter of set_sys_tune() (check) is 0.
>
>
> > The checks inside set_sys_tuen() can NOT guarantee the
> > "overcommit_memory" knob is exist or not, it only examines if the
> > value was being set correctly, because set_sys_tune has first use
> > SAFE_FILE_PRINTF which will TBROK directly when the knob non-exist.
>
> Ah, thanks for correcting me.
>
>
> > > > -     if (old_overcommit_ratio != -1)
> > > > -             set_sys_tune("overcommit_ratio", old_overcommit_ratio,
> 0);
> > > > -}
> > > > -
> > > >  static void overcommit_memory_test(void)
> > > >  {
>
> > > > @@ -269,6 +255,10 @@ static struct tst_test test = {
> > > >               {}
> > > >       },
> > > >       .setup = setup,
> > > > -     .cleanup = cleanup,
> > > >       .test_all = overcommit_memory_test,
> > > > +     .save_restore = (const struct tst_path_val[]) {
> > > > +             {"/proc/sys/vm/overcommit_memory", NULL, TST_SR_TBROK},
> > > > +             {"/proc/sys/vm/overcommit_ratio", NULL, TST_SR_TBROK},
> > > => shouldn't be here TST_SR_TCONF instead of TST_SR_TBROK?
>
>
> > It doesn't matter, I indeed consider this before, but after looking
> > through the rest mm tests they all use the function get|set_sys_tune()
> > which checks the knob mandatorily and run smoothly for past
> > many years and nobody ever complains about that.
>
> +1
>
> > So I think it's safe to convert this one using TBROK too, it essentially
> > has no difference from other oom-tests. 'overcommit_ratio' and
> > 'overcommit_memory' are quite basic on Linux distribution.
>
> +1
> => go ahead and merge.
>


Both pushed, thank you!


-- 
Regards,
Li Wang

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