Re: [Xen-devel] [PATCH 0/8] xen: don't let keyhandlers block indefinitely on locks

From: "Jürgen Groß" <jgross@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>,
	xen-devel@lists.xenproject.org
Cc: "Kevin Tian" <kevin.tian@intel.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>,
	"Jun Nakajima" <jun.nakajima@intel.com>, "Wei Liu" <wl@xen.org>,
	"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
	"George Dunlap" <George.Dunlap@eu.citrix.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	"Dario Faggioli" <dfaggioli@suse.com>,
	"Ross Lagerwall" <ross.lagerwall@citrix.com>,
	"Meng Xu" <mengxu@cis.upenn.edu>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH 0/8] xen: don't let keyhandlers block indefinitely on locks
Date: Fri, 14 Feb 2020 07:05:25 +0100	[thread overview]
Message-ID: <f4622e07-acad-7c2c-f061-e8ca8584a2d8@suse.com> (raw)
In-Reply-To: <1f5db5a6-e5c3-4d63-fdb4-e027d38bea18@citrix.com>

On 13.02.20 19:38, Andrew Cooper wrote:
> On 13/02/2020 12:54, Juergen Gross wrote:
>> Keyhandlers dumping hypervisor information to the console often need
>> to take locks while accessing data. In order to not block in case of
>> system inconsistencies it is convenient to use trylock variants when
>> obtaining the locks. On the other hand a busy system might easily
>> encounter held locks, so this patch series is adding special trylock
>> variants with a timeout used by keyhandlers.
> 
> This is a backwards step.
> 
> Keyhandlers are for debugging purposes.  When debugging it is far more 
> important to get the requested data, than almost anything else.

Right.

> 
> The system will cope with a multi-second outage occurring approximately 
> never.  A person debugging who can't get the data has no chance of 
> fixing whatever problem they are looking for.

Right.

> This series seems to be breaking the one critical usecase for 
> keyhandlers, to fix what - not let debugging get in the way of the 
> smooth running of the system?  A system in need of debugging in the 
> first place has bigger problems than needing to run smoothly.

Okay, this warrants a longer default timeout.

A keyhandler blocking on a lock will produce exactly no further data,
and it will probably block other keyhandlers, too, due to hogging at
least one cpu completely.

With a longer lock timeout (1 second?) there is a much higher chance
that the keyhandler will finish its job producing more data than
without any timeout.

BTW, during development of my core scheduling series I was hit by that
problem multiple times. With the lock timeout I'd have spared dozens of
reboots.

> The only thing which should happen to improve system stability is for 
> keyhandlers to disable the system watchdog while they are running, in 
> case they happen to run for seconds of wallclock time. This is an issue 
> which isn't addressed by the series, because once a keyhandler does get 
> a lock, it keeps it until it is done.

Right, will add disabling the watchdog during keyhandler action.

Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel