Hi Guenter,
Thanks for taking a look at this. Yes it's a double indirection, and reason to
do this is: I was worrying about if I use the system_wq to execute
'kernel_restart' (which calls to 'device_shutdown' and 'syscore_shutdown'), I
might end up blocking kernel modules' destructor callback function that also
using the system_wq for their cleaning up processes (although I didn't notice
there is a condition that a kernel module needs system_wq for its cleaning up).
To avoid resulting a blocking like this, I will need to execute
'kernel_restart' in a standalone context, that is: a kernel thread. But the
context to start up a kernel thread needs to be able to sleep, so this can't be
done in the execution context of the hrtimer callback function. Finally I have
to implement this as a double indirection: schedules a worker in hrtimer
callback function execution context, then creates a kernel thread in the worker
context, which is able to sleep.
And yes, thanks for catching this: both the worker and the kernel thread may
never be executed if there is a scheduling issue. I will upload v2 patch to
cover this by re-scheduling the same hrtimer with another TIMER_MARGIN (60
secs) expiration. So we can at least fallback to 'emergency_restart' when there
is a scheduling issue.
Sincerely,
Woody
On Wed, Jun 10, 2020 at 1:18 AM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> > pr_crit("Initiating system reboot\n");
> > + if (soft_reboot_target != NULL) {
> > + static DECLARE_WORK(reboot_work, reboot_work_fn);
> > +
> > + schedule_work(&reboot_work);
> > + return HRTIMER_NORESTART;
> > + }
>
> So this adds a double indirection: It first schedules a worker,
> then the worker creates a kernel thread, which finally calls
> kernel_restart(). If the softdog hangs, it may well be because
> of scheduling issues. If so, both the worker and the kernel thread
> may never execute, and the system would not reboot even though
> the softdog did fire.
>
> Is this double indirection really necessary ?
>
> Thanks,
> Guenter
