Re: [PATCH 06/24] perf daemon: Add config file support

From: Jiri Olsa
Date: Fri Feb 05 2021 - 19:16:00 EST


On Fri, Feb 05, 2021 at 09:14:54PM +0900, Namhyung Kim wrote:
> On Sun, Jan 31, 2021 at 8:49 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> [SNIP]
> > @@ -263,9 +605,16 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
> > signal(SIGTERM, sig_handler);
> >
> > while (!done && !err) {
> > - if (fdarray__poll(&fda, -1)) {
> > + err = daemon__reconfig(daemon);
>
> I think it's confusing since you put the reconfig function here.
> What not split normal and reconfig passes?

hum, not sure what's confusing in here? I've been known to
produce confusing code, but this one seems clear to me

>
> I mean something like below
>
> __cmd_start()
> {
> setup_server_config();
> daemon__run();

what's daemon__run? the daemon operates in the while loop below

>
> while (!done && !err) {
> ...
> if (reconfig) {
> daemon__kill();

you don't kill daemon for each reconfig change,
we detect changed sessions and kill/restart only them

> setup_server_config();
> daemon__reconfig();
> }
> }


so basically the current workflow is:

setup_server_config <--- reads config file, prepares session objects

while (!done) {
daemon__reconfig <--- check session objects states and run/stop them

if (fdarray__poll(&fda, -1)) {

handle_config_changes(&reconfig) <--- was there a config file change?

if (reconfig) <--- yes,
setup_server_config <--- change session objects/states
}
}

thanks,
jirka