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

[Jiri Olsa]

On Thu, Feb 11, 2021 at 03:01:12PM +0900, Namhyung Kim wrote:
> Hi Jiri,
> 
> On Tue, Feb 9, 2021 at 5:09 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> > +static int daemon__reconfig(struct daemon *daemon)
> > +{
> > +       struct daemon_session *session, *n;
> > +
> > +       list_for_each_entry_safe(session, n, &daemon->sessions, list) {
> > +               /* No change. */
> > +               if (session->state == OK)
> > +                       continue;
> > +
> > +               /* Remove session. */
> > +               if (session->state == KILL) {
> > +                       if (session->pid > 0) {
> > +                               daemon_session__kill(session);
> > +                               pr_info("reconfig: session '%s' killed\n", session->name);
> > +                       }
> > +                       daemon_session__remove(session);
> > +                       continue;
> > +               }
> > +
> > +               /* Reconfig session. */
> > +               if (session->pid > 0) {
> > +                       daemon_session__kill(session);
> > +                       pr_info("reconfig: session '%s' killed\n", session->name);
> > +               }
> > +               if (daemon_session__run(session, daemon))
> > +                       return -1;
> 
> Shouldn't it be 'continue'?  If there's a problematic session
> it'll prevent others from being processed.  And it seems this
> code will try to run it again and again.  Maybe we can put it
> in the KILL state (or a new FAILED state) IMHO.

so if there is misconfigured session, it will get executed,
and then we catch that it exited, like:

	# ./perf daemon start -f
	[2021-02-11 17:38:19.718166] daemon started (pid 1167439)
	[2021-02-11 17:38:19.719757] reconfig: ruining session [cycles:1167440]: abc -m 10M -e cycles --overwrite --switch-output -a
	[2021-02-11 17:38:19.720861] reconfig: ruining session [sched:1167441]: -m 20M -e sched:* --overwrite --switch-output -a
	[2021-02-11 17:38:21.132511] session 'cycles' exited, status=255

session will be removed

when the config is fixed and updated, daemon will pick it up:

	[2021-02-11 17:42:59.241140] reconfig: ruining session [cycles:1167642]: -m 10M -e cycles --overwrite --switch-output -a


daemon_session__run fails only there's no memory for allocation,
if mkdir fails (for other error than EEXIST) and if fork fails,
so pretty serious errors, where we want to bail out anyway

jirka