[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 4/8] xen: add locks with timeouts for keyhandlers

On 06.03.20 09:08, Jürgen Groß wrote:
On 05.03.20 16:25, Jan Beulich wrote:
On 13.02.2020 13:54, Juergen Gross wrote:
Most keyhandlers are used to dump hypervisor data to the console and
they are used mostly for debugging purposes. In those cases it might
happen that some data structures are locked and thus are blocking the
handler to access the data.

In order to be able to still get some information don't use plain
locking functions in the keyhandlers, but a variant of trylocks with
a timeout value. This allows to wait for some time and to give up in
case the lock was not obtained.

Add the main infrastructure for this feature including a new runtime
parameter allowing to specify the timeout value in milliseconds.

Use the new locking scheme in the handlers defined in keyhandler.c.

Personally I think trylock (as already used in some places) is the
way to go. Iirc others disagreed, but also didn't like the approach
taken here. I'm not intending to stand in the way if a majority
approves of this model, but I'm not going to ack these changes

Fair enough.

BTW, trylock is used at exactly three places: for dumping vNUMA, MSI and
livepatch info.

And TBH: the vNUMA case is really strange, as this is a rwlock which is
held as writer only in one place for a very brief time period when
freeing the domain's vnuma data.

The MSI case is more complicated and looking at it in more detail I've
realized that there is another trylock hidden in a subfunction:
vpci_msix_arch_print(). As vpci_msix_arch_print() will drop the lock in
the error case the interface to this function is rather weird. In
addition the occasional softirq processing is erro prone, too, as it
will happen only if a single domain has at least 64 MSI entries. In case
of lots of domains with up to 63 entries watchdog timeouts can still
happen, so I'll send a patch repairing this issue by letting
vpci_msix_arch_print() dump only one entry and putting the loop and
softirq handling into vpci_dump_msi().

Oh, sorry, softirqs are processed often enough, I missed one call.

Nevertheless the locking interface wants to be corrected IMO.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.