| Message ID | 20220113123418.1911231-1-liwang@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | lib: rewrite cgroup_find_ctrl with using for_each_ctrl | expand |
Hello Li, Li Wang <liwang@redhat.com> writes: > It is safe to start from controllers[0] to traverse each of > the controller whatever V2 or V1, then we can make use of it > in the cgroup_find_ctrl() function. Right, it seems we never set ctrl_root on "cgroup" nor is it added to ctrl_field. So it will be skipped in other loops. This might not be what people expect, but I'm not sure what to do about that. > > Signed-off-by: Li Wang <liwang@redhat.com> > --- > lib/tst_cgroup.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c > index 2ef599d9e..10b65364b 100644 > --- a/lib/tst_cgroup.c > +++ b/lib/tst_cgroup.c > @@ -214,7 +214,7 @@ static const char *cgroup_v2_ltp_mount = "unified"; > #define for_each_v1_root(r) \ > for ((r) = roots + 1; (r)->ver; (r)++) > #define for_each_ctrl(ctrl) \ > - for ((ctrl) = controllers + 1; (ctrl)->ctrl_name; (ctrl)++) > + for ((ctrl) = controllers; (ctrl)->ctrl_name; (ctrl)++) > > /* In all cases except one, this only loops once. > * > @@ -325,15 +325,14 @@ void tst_cgroup_print_config(void) > __attribute__ ((nonnull, warn_unused_result)) > static struct cgroup_ctrl *cgroup_find_ctrl(const char *const ctrl_name) > { > - struct cgroup_ctrl *ctrl = controllers; > - > - while (ctrl->ctrl_name && strcmp(ctrl_name, ctrl->ctrl_name)) > - ctrl++; > + struct cgroup_ctrl *ctrl; > > - if (!ctrl->ctrl_name) > - ctrl = NULL; > + for_each_ctrl(ctrl) { > + if (!strcmp(ctrl_name, ctrl->ctrl_name)) > + return ctrl; > + } > > - return ctrl; > + return NULL; > } > > /* Determine if a mounted cgroup hierarchy is unique and record it if so. Nice simplification! Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>
Richard Palethorpe <rpalethorpe@suse.de> wrote: > > It is safe to start from controllers[0] to traverse each of > > the controller whatever V2 or V1, then we can make use of it > > in the cgroup_find_ctrl() function. > > Right, it seems we never set ctrl_root on "cgroup" nor is it added to > ctrl_field. So it will be skipped in other loops. This might not be what > people expect, but I'm not sure what to do about that. Yes, but that's fine. It's because of the difference between V1 and V2. > > > > /* Determine if a mounted cgroup hierarchy is unique and record it if so. > > Nice simplification! > > Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com> Thanks for the review, I will merge this after the new release.
Hi Li,
LGTM.
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Kind regards,
Petr
Hi all, Patch merged, thanks! On Fri, Jan 21, 2022 at 1:23 PM Petr Vorel <pvorel@suse.cz> wrote: > > Hi Li, > > LGTM. > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > Kind regards, > Petr >
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c index 2ef599d9e..10b65364b 100644 --- a/lib/tst_cgroup.c +++ b/lib/tst_cgroup.c @@ -214,7 +214,7 @@ static const char *cgroup_v2_ltp_mount = "unified"; #define for_each_v1_root(r) \ for ((r) = roots + 1; (r)->ver; (r)++) #define for_each_ctrl(ctrl) \ - for ((ctrl) = controllers + 1; (ctrl)->ctrl_name; (ctrl)++) + for ((ctrl) = controllers; (ctrl)->ctrl_name; (ctrl)++) /* In all cases except one, this only loops once. * @@ -325,15 +325,14 @@ void tst_cgroup_print_config(void) __attribute__ ((nonnull, warn_unused_result)) static struct cgroup_ctrl *cgroup_find_ctrl(const char *const ctrl_name) { - struct cgroup_ctrl *ctrl = controllers; - - while (ctrl->ctrl_name && strcmp(ctrl_name, ctrl->ctrl_name)) - ctrl++; + struct cgroup_ctrl *ctrl; - if (!ctrl->ctrl_name) - ctrl = NULL; + for_each_ctrl(ctrl) { + if (!strcmp(ctrl_name, ctrl->ctrl_name)) + return ctrl; + } - return ctrl; + return NULL; } /* Determine if a mounted cgroup hierarchy is unique and record it if so.
It is safe to start from controllers[0] to traverse each of the controller whatever V2 or V1, then we can make use of it in the cgroup_find_ctrl() function. Signed-off-by: Li Wang <liwang@redhat.com> --- lib/tst_cgroup.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)