Re: [PATCH V3 6/9] virtio-ccw: implement synchronize_cbs()

From: Jason Wang
Date: Mon Apr 25 2022 - 23:43:33 EST

在 2022/4/26 11:38, Michael S. Tsirkin 写道:
On Mon, Apr 25, 2022 at 11:35:41PM -0400, Michael S. Tsirkin wrote:
On Tue, Apr 26, 2022 at 04:29:11AM +0200, Halil Pasic wrote:
On Mon, 25 Apr 2022 09:59:55 -0400
"Michael S. Tsirkin" <mst@xxxxxxxxxx> wrote:

On Mon, Apr 25, 2022 at 10:54:24AM +0200, Cornelia Huck wrote:
On Mon, Apr 25 2022, "Michael S. Tsirkin" <mst@xxxxxxxxxx> wrote:
On Mon, Apr 25, 2022 at 10:44:15AM +0800, Jason Wang wrote:
This patch tries to implement the synchronize_cbs() for ccw. For the
vring_interrupt() that is called via virtio_airq_handler(), the
synchronization is simply done via the airq_info's lock. For the
vring_interrupt() that is called via virtio_ccw_int_handler(), a per
device spinlock for irq is introduced ans used in the synchronization

Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: "Paul E. McKenney" <paulmck@xxxxxxxxxx>
Cc: Marc Zyngier <maz@xxxxxxxxxx>
Cc: Halil Pasic <pasic@xxxxxxxxxxxxx>
Cc: Cornelia Huck <cohuck@xxxxxxxxxx>
Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>

This is the only one that is giving me pause. Halil, Cornelia,
should we be concerned about the performance impact here?
Any chance it can be tested?
We can have a bunch of devices using the same airq structure, and the
sync cb creates a choke point, same as registering/unregistering.
BTW can callbacks for multiple VQs run on multiple CPUs at the moment?
I'm not sure I understand the question.

I do think we can have multiple CPUs that are executing some portion of
virtio_ccw_int_handler(). So I guess the answer is yes. Connie what do you think?

On the other hand we could also end up serializing synchronize_cbs()
calls for different devices if they happen to use the same airq_info. But
this probably was not your question

I am less concerned about synchronize_cbs being slow and more about
the slowdown in interrupt processing itself.

this patch serializes them on a spinlock.

Those could then pile up on the newly introduced spinlock.

Hmm yea ... not good.
Is there any other way to synchronize with all callbacks?

Maybe using rwlock as airq handler?