Date:   Mon, 10 Jan 2022 06:55:59 -0800
From:   David Vernet <void@manifault.com>
To:     Petr Mladek <pmladek@suse.com>
Cc:     Song Liu <song@kernel.org>, live-patching@vger.kernel.org,
        open list <linux-kernel@vger.kernel.org>, jpoimboe@redhat.com,
        jikos@kernel.org, mbenes@suse.cz, joe.lawrence@redhat.com
Subject: Re: [PATCH] livepatch: Avoid CPU hogging with cond_resched
Message-ID: <YdxI/xFf1btdIhLl@dev0025.ash9.facebook.com>
References: <20211229215646.830451-1-void@manifault.com>
 <CAPhsuW5PL1w_72Hrbsp2b3jA-SGyzv5oLfgybkq=s8J5KL6kmw@mail.gmail.com>
 <Ydf3MBet/B+lUdRv@alley>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Ydf3MBet/B+lUdRv@alley>
Precedence: bulk

Petr Mladek <pmladek@suse.com> wrote on Fri [2022-Jan-07 09:17:52 +0100]:
> On Thu 2022-01-06 16:21:18, Song Liu wrote:
> > PS: Do we observe livepatch takes a longer time to load after this change?
> > (I believe longer time shouldn't be a problem at all. Just curious.)
> 
> It should depend on the load of the system and the number of patched
> symbols. The module is typically loaded with a normal priority
> process.
> 
> The commit message talks about 1.3 seconds delay of ksoftirq. In
> principle, the change caused that this 1.3 sec of a single CPU time
> was interleaved with other scheduled tasks on the same CPU. I would
> expect that it prolonged the load just by a couple of seconds in
> the described use case.

Just to add a data point here for the record, I didn't observe any increase
in latency when applying a livepatch with this change. It took roughly ~2
+/- ~.5 seconds both with and without the change. Obviously that number
doesn't mean much without knowing the specifics of the patch and what
workloads were running on the host, but in general the invocations to
cond_resched() patch did not seem to materially affect the overhead of
livepatching.

The only time I would expect it to cause longer livepatches is when there
are a lot of tasks that need the CPU, which is dependent on system load as
Petr said. I'd argue that in this case, the patch would be working as
intended as hogging the CPU for 1 or more seconds seems like a recipe for
problems.