live-patching.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vasily Gorbik <gor@linux.ibm.com>
To: Petr Mladek <pmladek@suse.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>,
	Jiri Kosina <jikos@kernel.org>, Miroslav Benes <mbenes@suse.cz>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	Heiko Carstens <hca@linux.ibm.com>,
	Sven Schnelle <svens@linux.ibm.com>,
	Sumanth Korikkar <sumanthk@linux.ibm.com>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] livepatch: Kick idle cpu's tasks to perform transition
Date: Thu, 9 Sep 2021 10:54:05 +0200	[thread overview]
Message-ID: <your-ad-here.call-01631177645-ext-9742@work.hours> (raw)
In-Reply-To: <YSjgj+ZzOutFxevl@alley>

On Fri, Aug 27, 2021 at 02:54:39PM +0200, Petr Mladek wrote:
> On Wed 2021-07-07 14:49:38, Vasily Gorbik wrote:
> > --- a/kernel/livepatch/transition.c
> > +++ b/kernel/livepatch/transition.c
> > @@ -415,8 +415,11 @@ void klp_try_complete_transition(void)
> >  	for_each_possible_cpu(cpu) {
> >  		task = idle_task(cpu);
> >  		if (cpu_online(cpu)) {
> > -			if (!klp_try_switch_task(task))
> > +			if (!klp_try_switch_task(task)) {
> >  				complete = false;
> > +				set_tsk_need_resched(task);
> 
> Is this really needed?

Yes, otherwise the inner idle loop is not left and
klp_update_patch_state() is not reached. Only waking up idle
cpus is not enough.

> > +				kick_process(task);
> 
> This would probably do the job. Well, I wonder if the following is
> a bit cleaner.
> 
> 		wake_up_if_idle(cpu);

wake_up_if_idle() is nice way to identify idle cpus, but it does not
force idle task rescheduling in our case.

> Also, please do this in klp_send_signals(). We kick there all other
> tasks that block the transition for too long.

#define SIGNALS_TIMEOUT 15

Hm, kicking the idle threads in klp_send_signals() means extra 15 seconds
delay for every transition in our case and failing kselftests:

# --- expected
# +++ result
...
#  livepatch: 'test_klp_livepatch': starting patching transition
# +livepatch: signaling remaining tasks
#  livepatch: 'test_klp_livepatch': completing patching transition

BTW, for x86 I made a lousy tests with 128 cpus in kvm. With default
CONFIG_NO_HZ_IDLE=y kselftests are failing the same way. Sometimes
transition times out as well, despite NO_HZ options configurations.

Jul 09 11:43:33 q.q kernel: livepatch: 'test_klp_livepatch': starting patching transition
Jul 09 11:44:37 q.q kernel: livepatch: 'test_klp_livepatch': starting unpatching transition
Jul 09 11:45:40 q.q ERROR: failed to disable livepatch test_klp_livepatch

I understand this 15 seconds delay for loaded system and tasks doing real
work is good, but those lazy idle "running" tasks could be kicked right
away with no harm done, right? Given we are able to identify them reliably.
I'll send another patch version.

  reply	other threads:[~2021-09-09  8:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-07 12:49 Vasily Gorbik
2021-07-08 10:12 ` Petr Mladek
2021-08-06 23:59 ` Suraj Jitindar Singh
2021-08-27 12:54 ` Petr Mladek
2021-09-09  8:54   ` Vasily Gorbik [this message]
2021-09-10  8:08     ` Petr Mladek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=your-ad-here.call-01631177645-ext-9742@work.hours \
    --to=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=pmladek@suse.com \
    --cc=sumanthk@linux.ibm.com \
    --cc=svens@linux.ibm.com \
    --subject='Re: [RFC PATCH] livepatch: Kick idle cpu'\''s tasks to perform transition' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).