Re: [PATCH v2 2/6] tools/misc: rework xenwatchdogd signal handling

[Leigh Brown <leigh@xxxxxxxxxxxxx>]

Hi Andrew,

On 2024-04-11 13:12, Andrew Cooper wrote:
> On 29/03/2024 11:10 am, leigh@xxxxxxxxxxxxx wrote:
> > diff --git a/tools/misc/xenwatchdogd.c b/tools/misc/xenwatchdogd.c
> > index 2f7c822d61..35a0df655a 100644
> > --- a/tools/misc/xenwatchdogd.c
> > +++ b/tools/misc/xenwatchdogd.c
> > @@ -9,9 +9,11 @@
> >  #include <unistd.h>
> >  #include <signal.h>
> >  #include <stdio.h>
> > +#include <stdbool.h>
> >
> >  xc_interface *h;
> > -int id = 0;
> > +static bool safeexit = false;
> > +static bool done = false;
> It's a common bug, but these need to be volatile.  Right now, ...
I'm an idiot (I'm sure I've mentioned this a couple of times :-) )

> > @@ -90,10 +90,14 @@ int main(int argc, char **argv)
> >      if (id <= 0)
> >          err(EXIT_FAILURE, "xc_watchdog setup");
> >
> > -    for (;;) {
> > +    while (!done) {
> >          sleep(s);
> ... the only reason this isn't an infinite loop is because the compiler
> can't prove that sleep() doesn't modify the variable.  This goes wrong
> in subtle and fun ways when using LTO.
>
> I'll fix on commit.
>
> For the record, I've taken 1-3 which are ready.  You'll need to rework 
> 4
> and later per Anthony's feedback.
>
> ~Andrew
Thanks, I will get those done this evening, hopefully.

Regards,

Leigh.