On Tue, Apr 26, 2022 at 12:07:39PM +0800, Jason Wang wrote:

On Tue, Apr 26, 2022 at 11:55 AM Michael S. Tsirkin [off-list ref] wrote:

quoted

On Mon, Apr 25, 2022 at 11:53:24PM -0400, Michael S. Tsirkin wrote:

quoted

On Tue, Apr 26, 2022 at 11:42:45AM +0800, Jason Wang wrote:

quoted

在 2022/4/26 11:38, Michael S. Tsirkin 写道:

quoted

On Mon, Apr 25, 2022 at 11:35:41PM -0400, Michael S. Tsirkin wrote:

quoted

On Tue, Apr 26, 2022 at 04:29:11AM +0200, Halil Pasic wrote:

quoted

On Mon, 25 Apr 2022 09:59:55 -0400
"Michael S. Tsirkin" [off-list ref] wrote:

quoted

On Mon, Apr 25, 2022 at 10:54:24AM +0200, Cornelia Huck wrote:

quoted

On Mon, Apr 25 2022, "Michael S. Tsirkin" [off-list ref] wrote:

quoted

On Mon, Apr 25, 2022 at 10:44:15AM +0800, Jason Wang wrote:

quoted

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
method.

Cc: Thomas Gleixner <redacted>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Jason Wang <redacted>

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.

quoted

quoted

this patch serializes them on a spinlock.

Those could then pile up on the newly introduced spinlock.

Regards,
Halil

Hmm yea ... not good.

Is there any other way to synchronize with all callbacks?

Maybe using rwlock as airq handler?

Thanks

rwlock is still a shared cacheline bouncing between CPUs and
a bunch of ordering instructions.

Yes, but it should be faster than spinlocks anyhow.

quoted

quoted

Maybe something per-cpu + some IPIs to run things on all CPUs instead?

Is this something like a customized version of synchronzie_rcu_expedited()?

With interrupts running in an RCU read size critical section?
Quite possibly that is also an option.
This will need a bunch of documentation since this is not
a standard use of RCU, and probably get a confirmation
from RCU maintainers that whatever assumptions we make
are guaranteed to hold down the road.

quoted

... and I think classic and device interrupts are different enough
here ...

Yes.

Thanks

quoted

quoted

quoted

quoted

quoted

--
MST

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization