Re: [Xen-devel] [PATCH v2 3/4] xen: add process_pending_softirqs_norcu() for keyhandlers

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: "Jürgen Groß" <jgross@suse.com>
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>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Jan Beulich <jbeulich@suse.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [Xen-devel] [PATCH v2 3/4] xen: add process_pending_softirqs_norcu() for keyhandlers
Date: Mon, 24 Feb 2020 13:02:14 +0100	[thread overview]
Message-ID: <20200224120214.GN4679@Air-de-Roger> (raw)
In-Reply-To: <b01bd510-58c2-4a05-4e0a-dbf3ab6abd57@suse.com>

On Mon, Feb 24, 2020 at 12:44:48PM +0100, Jürgen Groß wrote:
> On 24.02.20 12:25, Roger Pau Monné wrote:
> > On Tue, Feb 18, 2020 at 01:21:13PM +0100, Juergen Gross wrote:
> > > Some keyhandlers are calling process_pending_softirqs() while holding
> > > a rcu_read_lock(). This is wrong, as process_pending_softirqs() might
> > > activate rcu calls which should not happen inside a rcu_read_lock().
> > 
> > It might be helpful to turn the ASSERT in process_pending_softirqs
> > into ASSERT_NOT_IN_ATOMIC also, as it would catch such missuses
> > AFAICT.
> 
> No, this would be triggering in __cpu_up() at system boot.

Yes, saw that in the next patch.

> > 
> > > 
> > > For that purpose add process_pending_softirqs_norcu() which will not
> > > do any rcu activity and use this for keyhandlers.
> > 
> > I wonder if for keyhandlers it might be easier to just disable the
> > watchdog in handle_keypress and remove the softirq processing from the
> > handlers.
> > 
> > At the end of day we want the keyhanders to run as fast as possible in
> > order to get the data out, and we only care about the watchdog not
> > triggering? (maybe I'm missing something here)
> 
> It is not that simple, I believe.
> 
> You'd need to be very careful that other functionality wouldn't suffer.
> I'm e.g. not sure time_calibration won't lead to a hanging system then.

AFAICT time_calibration is used to sync the timestamps of the various
CPUs so that they don't drift too much, but I don't think not
executing it could lead to a hang, it would lead to (bigger) skews
between CPUs, but such skews happen anyway.

Thanks, Roger.

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