On Tue 2023-02-28 08:56:08, Josh Poimboeuf wrote: > On Mon, Feb 27, 2023 at 04:55:47PM +0100, Petr Mladek wrote: > > On Fri 2023-02-24 08:50:00, Josh Poimboeuf wrote: > > > There have been reports [1][2] of live patches failing to complete > > > within a reasonable amount of time due to CPU-bound kthreads. > > > > > > Fix it by patching tasks in cond_resched(). > > > > > > There are four different flavors of cond_resched(), depending on the > > > kernel configuration. Hook into all of them. > > > > > > A more elegant solution might be to use a preempt notifier. However, > > > non-ORC unwinders can't unwind a preempted task reliably. > > > > > > [1] https://lore.kernel.org/lkml/20220507174628.2086373-1-song@xxxxxxxxxx/ > > > [2] https://lkml.kernel.org/lkml/20230120-vhost-klp-switching-v1-0-7c2b65519c43@xxxxxxxxxx > > > > > > Tested-by: Seth Forshee (DigitalOcean) <sforshee@xxxxxxxxxx> > > > Reviewed-by: Petr Mladek <pmladek@xxxxxxxx> > > > Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx> > > > > Just for record, I have double checked the changes against v2 > > and everything looks good to me. > > Whoops, so I found another little surprise: > > static int klp_check_stack(struct task_struct *task, const char **oldname) > { > static unsigned long entries[MAX_STACK_ENTRIES]; > ^^^^^^ > > That entries array is shared between the klp_mutex owner and all > cond_resched() callers. Huh, great catch! > MAX_STACK_ENTRIES is 100, which seems excessive. If we halved that, the > array would be "only" 400 bytes, which is *almost* reasonable to > allocate on the stack? It is just for the stack in the process context. Right? I think that I have never seen a stack with over 50 entries. And in the worst case, a bigger amount of entries would "just" result in a non-reliable stack which might be acceptable. It looks acceptable to me. > Alternatively we could have a percpu entries array... :-/ That said, percpu entries would be fine as well. It sounds like a good price for the livepatching feature. I think that livepatching is used on big systems anyway. I slightly prefer the per-cpu solution. Best Regards, Petr